-
Notifications
You must be signed in to change notification settings - Fork 343
Reduce Managed Lustre playback test time #1486
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR adds a PowerShell script to optimize Managed Lustre recorded test performance by removing consecutive "InProgress" polling responses, reducing test execution time from 15 minutes to under 1 minute.
Changes:
- Added
Fix-TestRecording.ps1script to process and optimize test recordings by deduplicating consecutive InProgress status responses - Updated
assets.jsonTag to reflect the new recording pushed to the assets repository
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 6 comments.
| File | Description |
|---|---|
| tools/Azure.Mcp.Tools.ManagedLustre/tests/Fix-TestRecording.ps1 | New PowerShell script that locates test recordings, removes consecutive InProgress responses, and pushes the optimized recording back to the assets repository |
| tools/Azure.Mcp.Tools.ManagedLustre/tests/Azure.Mcp.Tools.ManagedLustre.LiveTests/assets.json | Updated Tag value to reference the new optimized recording |
tools/Azure.Mcp.Tools.ManagedLustre/tests/Fix-TestRecording.ps1
Outdated
Show resolved
Hide resolved
tools/Azure.Mcp.Tools.ManagedLustre/tests/Fix-TestRecording.ps1
Outdated
Show resolved
Hide resolved
tools/Azure.Mcp.Tools.ManagedLustre/tests/Fix-TestRecording.ps1
Outdated
Show resolved
Hide resolved
Co-authored-by: Copilot <[email protected]>
…ng.ps1 (#1487) * Initial plan * Add documentation and remove invisible characters from Fix-TestRecording.ps1 Co-authored-by: hallipr <[email protected]> * Fix documentation to remove non-existent TestName parameter Co-authored-by: hallipr <[email protected]> * Apply suggestion from @hallipr * Update Fix-TestRecording.ps1 to remove PSEdition requirement Removed requirement for PSEdition Core in the script. --------- Co-authored-by: copilot-swe-agent[bot] <[email protected]> Co-authored-by: hallipr <[email protected]> Co-authored-by: Patrick Hallisey <[email protected]>
|
@copilot, from the assets.json change, can you generate a github tag comparison link so I can review the asset changes? |
|
The asset diff is: |
vcolin7
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
While I like that we now have a utility like this to remove some bloat from our recordings, I think we should still consider adding a way to customize the polling interval for clients we use during playback with Test Proxy like @alzimmermsft mentions in this issue. /cc @scbedd
weshaggard
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm ok with this bandaid for now. I would prefer this to be fixed in the test code in someway though instead of a post-recording process.
What does this PR do?
Managed Lustre recorded tests were talking a long time because they were polling over recorded "InProgress" responses. This script reduces consecutive InProgress responses and shortens the test time from 15 minutes to <1 minute
Pre-merge Checklist
servers/Azure.Mcp.Server/CHANGELOG.mdand/orservers/Fabric.Mcp.Server/CHANGELOG.mdfor product changes (features, bug fixes, UI/UX, updated dependencies)servers/Azure.Mcp.Server/README.mdand/orservers/Fabric.Mcp.Server/README.mddocumentationeng/scripts/Process-PackageReadMe.ps1. See Package README/servers/Azure.Mcp.Server/docs/azmcp-commands.mdand/or/docs/fabric-commands.md.\eng\scripts\Update-AzCommandsMetadata.ps1to update tool metadata in azmcp-commands.md (required for CI)ToolDescriptionEvaluatorand obtained a score of0.4or more and a top 3 ranking for all related test promptsconsolidated-tools.json/servers/Azure.Mcp.Server/docs/e2eTestPrompts.mdcrypto mining, spam, data exfiltration, etc.)/azp run mcp - pullrequest - liveto run Live Test Pipeline