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

Enhance tests #3

Merged
merged 3 commits into from
May 24, 2022
Merged

Enhance tests #3

merged 3 commits into from
May 24, 2022

Conversation

liquidnya
Copy link

Minor improvements to the test case source code.
This will allow to have test cases that have a log of more than 1 day (e.g. 3 days)
and also fixes some issues concerning time emulation as well as some issues with the emulated file system when testing a restart ("emulated crash") of the queue.

@ToransuShoujo ToransuShoujo merged commit 9d99b3c into ToransuShoujo:master May 24, 2022
@liquidnya
Copy link
Author

I also added some very basic and specific tests for weighted random.
Notably one of them (weight-remove.test.log) tests that the weighted random time is not removed from an incorrect user:
3f220f5#diff-f5ae652c672a0eb5c4e9845223e9f5ad1ae53526047dd1460e98bcd8717a4b56R46

In the other test case (weight-crash.test.log) a bug scenario is shown:
When the queue crashes after a wait time was changed (because someones level got picked) and before the 1 minute timer writes the new files to disk, then the wait time will be the previous one.
Even though this bug is shown, I suggest not fixing it, due to 3 reasons:

  • It is a very rare condition
  • In case it happens, it is in favor of the person who it happened to
  • Even if the wait time would be persisted whenever someone is removed from it's list,
    then the window of data being corrupt just gets smaller and does not fix the underlying issue

So I would suggest to eventually fix the underlying issue and to ignore those rare edge cases for now.

@ToransuShoujo
Copy link
Owner

@liquidnya
Copy link
Author

I've created the issue #12 for the underlying issue.

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.

2 participants