Only unregister unit test event listeners when **all** testing is complete#2431
Merged
Conversation
- Add test to ensure re-running failed tests count correctly
- also remove delay for `updatesetting` call (microsoft#2356)
Author
|
Also forcing #2356 (comment out artificial wait when we update settings during testing). In the debugger.test.ts we are most certainly skirting the timeout limit with this additional wait. |
- Ensure the promise chain passes results through - (was swallowing the value in the final promise chain to remove listeners) - Remove increased timeout limit in debugger.test
ericsnowcurrently
approved these changes
Aug 22, 2018
| // Test everything. | ||
| if (testPaths.length === 0) { | ||
| await runTestInternal(); | ||
| await this.removeListenersAfter(runTestInternal()); |
There was a problem hiding this comment.
FYI, it took me a sec to realize what happened to runTestInternal(). :) I'm not a huge fan of cramming several things onto one line (even if it's the Javascript way ). It hurts readability. Feel free to clean this up. I'll leave it up to you. :)
| suiteSetup(async () => { | ||
| // Test disvovery is where the delay is, hence give 10 seconds (as we discover tests at least twice in each test). | ||
| // tslint:disable-next-line:no-invalid-this | ||
| this.timeout(10000); |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Addendum to fixes for #2143
Add test to ensure re-running failed tests count correctly
We use promise-chains for 'queuing' up our tests when they span multiple files/suites/functions, add one last member to the chain that un-registers the event handlers.
Title summarizes what is changing
Has a news entry file (Still using the originally created news file)
Has unit tests & system/integration tests