Add AdminController Test Suite #187
Merged
Conversation
This file contains 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
|
Heads up, you need to rebase (merge conflict). |
Squashed set of commits for 6f51db8...c70e444. Merge conflict was on package-lock, which was hard to fix, so I just redid the commits.
StormFireFox1
force-pushed
the
add-admin-test-suite
branch
from
c70e444 to
cba950b
Compare
|
Just did a squash on top of a rebase, since the merge conflict broke the lockfile pretty bad. I'll keep this as a draft for another day or two, since I might be able to come up with more tests for the suite. |
StormFireFox1
requested review from
shravanhariharan2 and
sumeet-bansal
as code owners
Test is not necessary due to lack of filtering in original controller.
Tests had basically the same boilerplate. Combine them to check both conditions at once. The test already checked if we were adding bonus point to _specifically_ the emails we passed in, so it's best to also check if an extra user wasn't included at the same time.
Inlining UserController made one line too long. Since it is used multiple times in the test, extract variable to make its usage less verbose.
shravanhariharan2
approved these changes
Jun 20, 2021
ActivityModel is missing the `pointsEarned` field whenever creating an Activity after attending an event. This only happens in our mock tester. Add a check for this field's presence, along with fixing the inherent bug within the mock state.
sumeet-bansal
approved these changes
Jul 6, 2021
|
Due to a bug in the factory code causing events to randomly have start times before account creation times, it's possible for non-deterministic test suite failures because of account creation occurring after event attendance. This bug is (supposedly) solved by PR #181 including the |
|
Here's an example - the event attendance timestamp occurs before the user creation since presumably, the event is randomly created in the past and has its timestamp set to a random time between start and end date. See https://github.com/acmucsd/membership-portal/blob/master/tests/data/PortalState.ts#L88 |
|
@StormFireFox1 That PR hasn't been updated in a month and has changes requested, so let's just copy those factory changes over to this PR and merge this. I don't think there shouldn't be any merge conflicts, but even if there are, #181 can handle them. |
added 2 commits
July 16, 2021 17:03
Account creation can occur after event attendance, causing activities to be out of order chronologically. Guarantee order in activities by introducing retroactive attendance in a future event. Bug in factory is not a reproducible issue in production, since account creation will always occur before event attendance in real time.
|
Actually @StormFireFox1 this probably affects a bunch of other tests too. We could just set the timestamp of the account creation activity to like a month ago to ensure that activity always happens first. |
|
I figured yesterday explicitly setting account creation earlier is safer. I'll readjust all this tonight. |
Mock factory `write` function incorrectly saves attendance before any possible activities, typically causing event attendances to be stored in the database prior to activities being stored. Adjust order of flushing to database in order to consistently set smaller timestamps for account-related activities, such as account creation.
This reverts commit b5f03dc. Tests continue to be non-deterministic despite this change. My assumption was wrong.
Create users at beginning of the Unix epoch to ensure guarantee of event attendance activity being second is fulfilled. Since Activities are sorted by timestamp, we need to guarantee account creation does not occur before attendance of event. Fix issue with above workaround, providing explicit method whenever necessary.
Rather than having two explicit methods, merge methods and use optional timestamp argument to allow for custom creation of users at explicit times.
Use specified timestamp of 1 month before current time to place account creation in time before most activities for testing.
nick-ls
pushed a commit
to nick-ls/membership-portal
that referenced
this pull request
Aug 6, 2024
* Add tests for "/bonus" and "/emails" routes Squashed set of commits for 6f51db8...c70e444. Merge conflict was on package-lock, which was hard to fix, so I just redid the commits. * nit: "proxyUser" -> "adminUser" for admin tests * Adjust array equality checks on admin tests * Remove test for non-present emails Test is not necessary due to lack of filtering in original controller. * Inline functions and variables in admin tests * Modify test to check bonus points for all users * Combine both bonus points tests into one Tests had basically the same boilerplate. Combine them to check both conditions at once. The test already checked if we were adding bonus point to _specifically_ the emails we passed in, so it's best to also check if an extra user wasn't included at the same time. * Rearrange asserts to suffix response calls * Rename adminUser to admin in admin test suite * Adjust order of assertions in admin test * Adjust name of extraneous user in admin test * Make variable for UserController in admin test Inlining UserController made one line too long. Since it is used multiple times in the test, extract variable to make its usage less verbose. * Fix nits and check attendance response details * Check for correct activities in admin test suite * Add points field of ActivityModel in test suite ActivityModel is missing the `pointsEarned` field whenever creating an Activity after attending an event. This only happens in our mock tester. Add a check for this field's presence, along with fixing the inherent bug within the mock state. * Port EventFactory code from acmucsd#181 Code from acmucsd#181 adds ability to fake events in the past, future or present. Taken verbatim from commit 18a952c. * Adjust test suite to account for factory bug Account creation can occur after event attendance, causing activities to be out of order chronologically. Guarantee order in activities by introducing retroactive attendance in a future event. Bug in factory is not a reproducible issue in production, since account creation will always occur before event attendance in real time. * Revert event factory code changes * Adjust transaction order when saving factory state Mock factory `write` function incorrectly saves attendance before any possible activities, typically causing event attendances to be stored in the database prior to activities being stored. Adjust order of flushing to database in order to consistently set smaller timestamps for account-related activities, such as account creation. * Revert transaction order adjustment This reverts commit b5f03dc. Tests continue to be non-deterministic despite this change. My assumption was wrong. * Guarantee correct activity order in failed tests Create users at beginning of the Unix epoch to ensure guarantee of event attendance activity being second is fulfilled. Since Activities are sorted by timestamp, we need to guarantee account creation does not occur before attendance of event. Fix issue with above workaround, providing explicit method whenever necessary. * Merge user creation methods to have optional args Rather than having two explicit methods, merge methods and use optional timestamp argument to allow for custom creation of users at explicit times. * Adjust user creation method to have set timestamp Use specified timestamp of 1 month before current time to place account creation in time before most activities for testing. * Fix argument call for creating users in test suite Co-authored-by: Matei-Alexandru Gardus <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Closes #174.