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.

Sign up for GitHub

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

Jump to bottom

Add unittests for models #1416

Merged

quantum5 merged 2 commits into DMOJ:master from Ninjaclasher:models-testing

Jun 13, 2020

Member

Ninjaclasher commented Jun 2, 2020

No description provided.

Ninjaclasher force-pushed the models-testing branch 3 times, most recently from 8d0dade to 32a36de Compare

June 2, 2020 03:34

codecov-commenter commented Jun 2, 2020 •

edited

Loading

Codecov Report

Merging #1416 into master will increase coverage by 7.69%.
The diff coverage is 99.36%.

@@            Coverage Diff             @@
##           master    #1416      +/-   ##
==========================================
+ Coverage   37.92%   45.62%   +7.69%     
==========================================
  Files         198      207       +9     
  Lines       11337    11872     +535     
==========================================
+ Hits         4300     5417    +1117     
+ Misses       7037     6455     -582

Impacted Files	Coverage Δ
judge/models/tests/util.py	`97.50% <97.50%> (ø)`
judge/models/tests/test_blogpost.py	`100.00% <100.00%> (ø)`
judge/models/tests/test_contest.py	`100.00% <100.00%> (ø)`
judge/models/tests/test_problem.py	`100.00% <100.00%> (ø)`
judge/models/tests/test_profile.py	`100.00% <100.00%> (ø)`
judge/models/tests/test_submission.py	`100.00% <100.00%> (ø)`
judge/admin/problem.py	`57.23% <0.00%> (-2.21%)`	⬇️
judge/models/comment.py	`38.81% <0.00%> (-0.79%)`	⬇️
... and 47 more

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 c6e7a38...185eed7. Read the comment docs.

Ninjaclasher force-pushed the models-testing branch 7 times, most recently from fc08a80 to f82908b Compare

June 2, 2020 17:25

Member Author

Ninjaclasher commented Jun 2, 2020 •

edited

Loading

Todo:

~~Add tests for Comment~~
Add fixture (?) for users, profiles, permissions, organizations
Fix wrong Problem.is_accessible_by method
Reduce the amount of copy pasting for Contest, Problem, and BlogPost method testing

Ninjaclasher force-pushed the models-testing branch from e8e22d0 to f530d65 Compare

June 2, 2020 20:16

Ninjaclasher mentioned this pull request

Make Problem.is_accessible_by consistent with Problem.get_visible_problems #1420

Merged

Ninjaclasher force-pushed the models-testing branch 5 times, most recently from 1e79d70 to c54d442 Compare

June 2, 2020 23:58

Ninjaclasher mentioned this pull request

Fix Submission.can_see_detail erroring when user is not authenticated #1423

Merged

Ninjaclasher force-pushed the models-testing branch 7 times, most recently from 5bacadf to 02bdbc3 Compare

June 3, 2020 17:56

Ninjaclasher marked this pull request as ready for review

June 3, 2020 18:00

Member Author

Ninjaclasher commented Jun 3, 2020

This PR covers unittests for most of the models.

Models excluded:

Comments (this will be done in a future PR)
Problem Data (this should probably just be done alongside unittests for the views)
Judge/Runtime (this requires a running bridge/judges for thorough testing)

fluix-dev reviewed

View reviewed changes

judge/models/tests/test_submission.py Outdated Show resolved Hide resolved

judge/models/tests/util.py Outdated Show resolved Hide resolved

Ninjaclasher force-pushed the models-testing branch 2 times, most recently from ffc9bb8 to c20ec28 Compare

June 3, 2020 18:21

quantum5 reviewed

View reviewed changes

judge/models/tests/util.py Outdated Show resolved Hide resolved

judge/models/tests/util.py Outdated Show resolved Hide resolved


          Add unittests for most of the models

be6f81d

Ninjaclasher force-pushed the models-testing branch from e897018 to b8ab7fb Compare

June 4, 2020 01:10

Ninjaclasher requested a review from quantum5

June 4, 2020 01:12

fluix-dev suggested changes

View reviewed changes

Contributor

fluix-dev left a comment

Otherwise looks great!

judge/models/tests/test_contest.py Outdated Show resolved Hide resolved

judge/models/tests/test_contest.py Outdated Show resolved Hide resolved

judge/models/tests/test_profile.py Show resolved Hide resolved

judge/models/tests/test_contest.py Show resolved Hide resolved

judge/models/tests/test_contest.py Show resolved Hide resolved

judge/models/tests/test_problem.py Show resolved Hide resolved

judge/models/tests/test_profile.py Show resolved Hide resolved

judge/models/tests/test_contest.py Show resolved Hide resolved

quantum5 reviewed

View reviewed changes

judge/models/tests/test_contest.py Outdated

+                  def test_basic_contest(self):
+                      self.assertTrue(self.basic_contest.show_scoreboard)
+                      self.assertIsInstance(self.basic_contest.contest_window_length, timezone.timedelta)

Member

quantum5 Jun 4, 2020

Why not assert the actual values?

Member Author

Ninjaclasher Jun 4, 2020

Done for this (since the timedelta is fixed). However, I don't think we should assert the actual values for the ones that are dependant on the current time (e.g time_remaining).

Contributor

fluix-dev Jun 5, 2020

Could you not get an accurate calculation by using the Contest's _now cached property?

Member Author

Ninjaclasher Jun 7, 2020

Personally, I don't think it is worth it to check for the exact match in these cases. If you're using the Contest's _now property, you basically end up rewriting that method's calculation as a test, which doesn't really test what the method "should" return.

judge/models/tests/test_contest.py Outdated Show resolved Hide resolved

Ninjaclasher force-pushed the models-testing branch from b8ab7fb to 2380f2f Compare

June 4, 2020 03:23


          Requested changes

185eed7

Ninjaclasher force-pushed the models-testing branch from 0f1c76b to 185eed7 Compare

June 5, 2020 14:10

Ninjaclasher requested a review from quantum5

June 5, 2020 14:10

fluix-dev approved these changes

View reviewed changes

Contributor

fluix-dev left a comment

LGTM

quantum5 approved these changes

View reviewed changes

quantum5 merged commit 3126dca into DMOJ:master

Ninjaclasher deleted the models-testing branch

June 28, 2020 23:24

outloudvi pushed a commit to SchOJ/site that referenced this pull request


          Add unittests for models (DMOJ#1416)

70d7cd0

malbareda pushed a commit to malbareda/JOEL-web that referenced this pull request


          Add unittests for models (DMOJ#1416)

32d02a1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet