Fix test stubbings of global XHR object #489
Merged
Conversation
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
XMLHttpRequest is stubbed by sinon but never assigned to the global! Thus, the stubbing is never used anywhere in this test suite _except_ that it relies on the stubbing done in _other_ test files that aren't cleaned up. This ensures the stubbing is set and used within this test file. (But does not clean up the bad global stubbings that are left intact after each test run.) This partially fixes optimizely#488
This stubbing is created and replaces the global value, but was not being cleaned up after each run! Thus, the stubbing side effect was leaking to other tests. :( This ensures the stubbing is removed after each test so subsequent tests are run cleanly.
Any variable that is declared outside of a test will leak state between tests. This is bad and dangerous. Variables that are necessary to reference between both setup and test callbacks should be defined on the test context, which mocha creates fresh for each test. This way, cross-test pollution is minimized. (This is part of the problem that caused: optimizely#488)
There is no need to "restore" a stubbing via sinon when the entire thing will be deleted. Also, many of these tests aren't asserting on the requests array so there is no use for it.
mjc1283
approved these changes
May 28, 2020
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Summary
Fixes cross-test pollution of global XHR object.
The global XHR object was being stubbed in some tests but not others. But the tests that didn't stub weren't failing because the tests that were stubbing weren't being cleaned up! So some test suites were depending (implicitly) on side effects from other test files.
Issues
partially fixes #488