fix: some validateAdminArgs unit tests wouldn't actually fail under incorrect behavior #3009
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
some of the
validateAdminArgs()unit tests used a pattern of nested check-expects, and the outer expect would just be checking that a specific error wasn't thrown, which would swallow up any failures from inner expects - it would always pass the outer expect, meaning that even if the inner expects should fail the test, the test would pass.this is shown on lines 224 of
validate-admin-args.spec.ts, where variables are provided tovalidateAdminArgs()but on line 227 we check that they areundefined. if you bring this incorrect inner expect out of the outer expect, the test fails - because we are expectingundefinedwhen in reality we passed in variables and are correctly returned variables. to be clear, this is an incorrectly written test, but the behavior of the function is correct - the bad test wasn't caught because of this nested check-expects pattern.changing the testing pattern here to flatten the expects to the same level solves the problem!