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

Add some unit tests #3678

Open
wants to merge 6 commits into
base: main
Choose a base branch
from
Open

Add some unit tests #3678

wants to merge 6 commits into from

Conversation

user1823
Copy link
Contributor

Covers most of the cases encountered in #3639

Covers most of the cases encountered in ankitects#3639
@user1823
Copy link
Contributor Author

Not sure why is the test failing.

In the following test, the left side should produce 2 items. But CI failure shows 3 items on left side.

assert_eq!(
convert(
&[
revlog(RevlogReviewKind::Review, 9),
RevlogEntry {
ease_factor: 0,
..revlog(RevlogReviewKind::Manual, 7)
},
revlog(RevlogReviewKind::Review, 1),
revlog(RevlogReviewKind::Relearning, 0),
],
false,
),
fsrs_items!([review(0), review(1)])
);

Makes the test more robust.
@user1823
Copy link
Contributor Author

@L-M-Sherlock, can you please have a look when you are free?

@L-M-Sherlock
Copy link
Contributor

The output of CI showed that the test was failed:

--- STDERR:              anki scheduler::fsrs::params::tests::card_reset_drops_all_previous_history ---
--
  | thread 'scheduler::fsrs::params::tests::card_reset_drops_all_previous_history' panicked at rslib/src/scheduler/fsrs/params.rs:602:9:
  | assertion `left == right` failed
  | left: Some([FSRSItem { reviews: [FSRSReview { rating: 3, delta_t: 0 }] }, FSRSItem { reviews: [FSRSReview { rating: 3, delta_t: 0 }, FSRSReview { rating: 3, delta_t: 1 }] }])
  | right: Some([FSRSItem { reviews: [FSRSReview { rating: 3, delta_t: 0 }, FSRSReview { rating: 3, delta_t: 1 }] }])
  | note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

When training, the first FSRS item is removed. That's why none of the other tests includes it.

Co-authored-by: Jarrett Ye <[email protected]>
@user1823
Copy link
Contributor Author

user1823 commented Jan 1, 2025

Thanks @L-M-Sherlock

@@ -636,4 +689,20 @@ pub(crate) mod tests {
convert(revlogs, false)
);
}

#[test]
fn even_if_no_learning_steps_ignore_reviews_before_during_reviewing() {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Just a naming nitpick:

Suggested change
fn even_if_no_learning_steps_ignore_reviews_before_during_reviewing() {
fn ignore_reviews_before_during_reviewing_even_if_no_learning_steps() {

@user1823
Copy link
Contributor Author

user1823 commented Jan 2, 2025

Hope that you like the wording now.

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.

3 participants