Skip to content
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

tests: increase clarity by a marginal amount, **rm api and tests/api** #583

Merged
merged 3 commits into from
Oct 29, 2020

Conversation

chuck-sys
Copy link
Collaborator

@chuck-sys chuck-sys commented Oct 29, 2020

Ticket(s)

Closes #531

Details

Since I am reasonably satisfied with the size of the tests (for some reason even after I made the PR from last time this still stays), I will close this. Should have closed this like 23 days ago.

  • rename deep-dive to user inspect because we are using --inspect
  • remove most self.assertEqual(code, 200) because there is no other possible code that you can actually output (so no testing)
  • remove api and tests/api to alleviate confusion on inconsistency between command logics

@chuck-sys chuck-sys requested review from a team as code owners October 29, 2020 07:06
@codecov
Copy link

codecov bot commented Oct 29, 2020

Codecov Report

Merging #583 into master will decrease coverage by 0.53%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #583      +/-   ##
==========================================
- Coverage   93.54%   93.01%   -0.54%     
==========================================
  Files          47       42       -5     
  Lines        2712     2420     -292     
  Branches      370      323      -47     
==========================================
- Hits         2537     2251     -286     
+ Misses        115      110       -5     
+ Partials       60       59       -1     
Impacted Files Coverage Δ
app/controller/command/commands/user.py 98.93% <100.00%> (ø)
db/utils.py 70.83% <0.00%> (-16.67%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 97f98f7...267d677. Read the comment docs.

Slack commands by default should always give 200 codes, meaning that
any other code is pointless. It represents that the message was well
received by the other party. Replacing it with an underscore retains
clarity.
@chuck-sys chuck-sys changed the title tests: rename from deepdive to inspect for clarity tests: increase clarity by a marginal amount Oct 29, 2020
@chuck-sys chuck-sys changed the title tests: increase clarity by a marginal amount tests: increase clarity by a marginal amount, **rm api and tests/api** Oct 29, 2020
Copy link
Member

@bobheadxi bobheadxi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

im not entirely sure what happened here, but i trust it's a good decision, and similar coverage using less tests seems alright :)

@chuck-sys chuck-sys merged commit a5c1323 into master Oct 29, 2020
@chuck-sys chuck-sys deleted the cheukyin699/#531-further-improve-testing branch October 29, 2020 15:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Testing isn't what it seems to be
2 participants