Conversation
There was a problem hiding this comment.
Pull request overview
This PR fixes issues with long-running nightly tests by ensuring the Azure DevOps subscription provider is properly authenticated and by handling race conditions during test setup.
Changes:
- Added explicit sign-in step to Azure DevOps subscription provider setup to ensure authentication is complete before tests run
- Enhanced test setup to re-establish the provider and handle race conditions with tree refresh operations
- Added defensive null-check for outputChannel in error handling to prevent crashes during test initialization
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| test/utils/azureDevOpsSubscriptionProvider.ts | Added sign-in step after provider creation to ensure authentication is established before tests use the provider |
| test/nightly/crud.test.ts | Re-establishes AzDO provider after clearing mock overrides, adds tree refresh synchronization, and improves subscription filtering to skip tests gracefully when no subscriptions are available |
| src/utils/wrapFunctionsInTelemetry.ts | Added optional chaining to outputChannel access to handle edge cases where it might not be initialized |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // the logIn command in global.nightly.test.ts) cancels our getChildren() call | ||
| // via the shared cancellation token in AzureResourceTreeDataProvider. | ||
| await vscode.commands.executeCommand('azureResourceGroups.refresh'); | ||
| await new Promise(resolve => setTimeout(resolve, 2000)); |
There was a problem hiding this comment.
The hard-coded 2000ms timeout is a potential source of test flakiness. If the tree refresh takes longer than expected (due to slow CI or other factors), the test may fail intermittently. Consider alternatives such as polling for the tree state or using an event-based approach to detect when the refresh is complete. If a hard-coded timeout is necessary, consider making it configurable via an environment variable or increasing it to a more conservative value.
2 participants
Make sure we are using the AzDevOps provider whenever we are doing the crud tests.