-
-
Notifications
You must be signed in to change notification settings - Fork 4.4k
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
Enhancements #531
Enhancements #531
Conversation
DeepCode's analysis on #f93fc4 found:
💬 This comment has been generated by the DeepCode bot, installed by the owner of the repository. The DeepCode bot protects your repository by detecting and commenting on security vulnerabilities or other critical issues. |
I suspect @nickpalenchar planned on a nested tests kinda thing for this. At least those naming looks like that. Anyway, only merge at Nick's review approval. |
Hey @jamesgeorge007, TravisBuddy Request Identifier: d0e3e9d0-4109-11ea-a60c-ed450572c3ce |
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.
Thanks for these changes @jamesgeorge007, especially with proofreading my frantic typing mishaps! This change sacrifices some readability of the testing API. toBeLevel(200)
sounds like a status should be that exactly when reading it like English. Furthermore, it increases margin for accidental false positives. E.g if a user tried with toBeLevel(404)
, the test would pass if the server 500'd in most situations. the toBeLevelxxx
methods are higher-level convince and are unlikely to scale past 4 different ranges anytime soon in the land of http. I'm fine with the redundancy at that scale, as well as finding ways to DRY it up, but it should not sacrifice obvious readability or simple usage.
The corrections of my js misusage (let
-> const
, typos) should def get merged regardless.
247215b
to
a55eb11
Compare
Hey @jamesgeorge007, TravisBuddy Request Identifier: aa57e3e0-412a-11ea-a60c-ed450572c3ce |
Hey @jamesgeorge007, TravisBuddy Request Identifier: d5f226f0-412a-11ea-a60c-ed450572c3ce |
Hey @jamesgeorge007, TravisBuddy Request Identifier: fd5ddb80-412a-11ea-a60c-ed450572c3ce |
Hey @jamesgeorge007, TravisBuddy Request Identifier: 51c4f780-4130-11ea-a60c-ed450572c3ce |
Follow up of #518