Switch to using the Test2::Suite and Test2::MojoX modules for unit tests. #119
Conversation
|
Note that this requires version 0.000139 or newer of Test2::Suite since it uses check_isa in the tests. The version on Ubuntu 20.04 is 0.000129, so an install from cpan is needed there. The version on Ubuntu 22.04 is 0.000144, and so is new enough. I need to upgrade. |
drgrice1
force-pushed
the
perl-unit-tests-with-test2-suite
branch
2 times, most recently
from
17ea746
to
d2d5888
Compare
| dies { | ||
| $user_rs->addCourseUser( | ||
| info => { course_name => 'Topology', username => 'quimby' }, | ||
| params => { role => 'student', course_user_params => { useMathQuill => 0 } } |
There was a problem hiding this comment.
useMathQuill is supposed to be a boolean. Switch to false?
There was a problem hiding this comment.
This is supposed to be 0. That is the point of this test. It is checking that an invalid field is passed in the parameters. If this is switched to false an exception will not be thrown because that is valid.
| @@ -124,8 +124,8 @@ $t->get_ok('/webwork3/api/courses/4/settings')->status_is(200)->content_type_is( | |||
| $t->post_ok('/webwork3/api/courses' => json => $new_course)->status_is(403)->json_is('/has_permission' => 0); | |||
|
|
|||
| $t->put_ok('/webwork3/api/courses/4' => json => { course_name => 'XXX' })->status_is(403) | |||
| ->json_is('/has_permission' => false); | |||
| ->json_is('/has_permission' => 0); | |||
There was a problem hiding this comment.
This seems like this should be a boolean and can then this with a switch on line 95 in WeBWorK3::Controller::Permission to has_permission => false.
There was a problem hiding this comment.
This would then affect almost all of the other mojolicious tests as well though.
There was a problem hiding this comment.
I am not going to change this in this pull request. This was changed to 0 because currently the method returns a numeric (not boolean) value. That is a TODO for later.
| params => { | ||
| course_name => 'Precalculus', | ||
| set_name => 'HW #11', | ||
| set_dates => { open => 100, due => 140, answer => 200, enable_reduced_scoring => 0 }, |
There was a problem hiding this comment.
enable_reduced_scoring => false
There was a problem hiding this comment.
Again, this needs to be 0. That is the point of this test.
There was a problem hiding this comment.
Although, it seems that set_params => { hide_hint => 0 } should be false instead of 0 in this test so that the enable_reduced_scoring value is appropriately tested (the previous test already checks the set_params values.
|
Overall, this looks great. Good cleanup and improved functionality. Just added a couple of minor items in the code. |
|
It sounds like we will need a separate PR that standardizes the use of booleans (and corresponding cleanup to the DB to ensure that our boolean fields are properly typed). I'll add this to #110 |
|
I can make the change to permissions in this pull request if you want me to. However, the objective of this pull request was to make the switch to Test2::Suite and Test2::MojoX, and not to change the current behavior. I had observed that the |
|
I'm fine merging in and then doing some cleanup, especially with booleans. That's probably the better way to go. |
drgrice1
force-pushed
the
perl-unit-tests-with-test2-suite
branch
5 times, most recently
from
1bac71e
to
5644c16
Compare
This uses Ubuntu 22.04 for the runner and docker image.
drgrice1
force-pushed
the
perl-unit-tests-with-test2-suite
branch
4 times, most recently
from
1d70fe2
to
dd8303e
Compare
…sts. Note that is important for the unit tests to have the `got` argument first and the `expected` argument second in an `is` test. Test2::Tools::Compare converts the second `expected` argument into a Test2::Tools object which is important for strict checks. Also remove the incomplete t/db/test_course_user.pl file.
drgrice1
force-pushed
the
perl-unit-tests-with-test2-suite
branch
from
dd8303e
to
eec4b19
Compare
Note that is important for the unit tests to have the
gotargument first and theexpectedargument second in anistest. Test2::Tools::Compare converts the secondexpectedargument into a Test2::Tools object which is important for strict checks.Also remove the incomplete t/db/test_course_user.pl file.
Note that this depends on #117.