From 390a8421aa14181f2b183b2499bf3116cddac8a5 Mon Sep 17 00:00:00 2001 From: Damien Elmes Date: Fri, 14 May 2021 22:16:53 +1000 Subject: [PATCH] fix test scheduler undo + implement look-ahead Instead of using a separate undo queue, the code now defers checking for newly-due learning cards until the answering stage, and logs the updated cutoff time as an undoable change, so that any newly-due learning cards won't appear instead of a new/review card that was just undone. Queue redo now uses a similar approach to undo, instead of rebuilding the queues. --- qt/aqt/operations/__init__.py | 2 - rslib/src/backend/dbproxy.rs | 2 +- rslib/src/backend/scheduler/mod.rs | 4 +- rslib/src/lib.rs | 8 ++ rslib/src/scheduler/answering/mod.rs | 2 +- rslib/src/scheduler/queue/builder/mod.rs | 7 +- rslib/src/scheduler/queue/entry.rs | 12 ++ rslib/src/scheduler/queue/learning.rs | 157 +++++++++++++--------- rslib/src/scheduler/queue/main.rs | 30 +++-- rslib/src/scheduler/queue/mod.rs | 159 +++++++++++++---------- rslib/src/scheduler/queue/undo.rs | 125 ++++++------------ rslib/src/timestamp.rs | 9 +- rslib/src/undo/mod.rs | 2 - 13 files changed, 278 insertions(+), 241 deletions(-) diff --git a/qt/aqt/operations/__init__.py b/qt/aqt/operations/__init__.py index 2d1e9921c..4c5d1db35 100644 --- a/qt/aqt/operations/__init__.py +++ b/qt/aqt/operations/__init__.py @@ -134,8 +134,6 @@ class CollectionOp(Generic[ResultWithChanges]): changes = result.changes # fire new hook - print("op changes:") - print(changes) aqt.gui_hooks.operation_did_execute(changes, handler) # fire legacy hook so old code notices changes if mw.col.op_made_changes(changes): diff --git a/rslib/src/backend/dbproxy.rs b/rslib/src/backend/dbproxy.rs index c9dba9c13..125fb1585 100644 --- a/rslib/src/backend/dbproxy.rs +++ b/rslib/src/backend/dbproxy.rs @@ -111,7 +111,7 @@ pub(super) fn db_command_bytes(col: &mut Collection, input: &[u8]) -> Result Result { - self.with_col(|col| col.get_queued_cards(input.fetch_limit, input.intraday_learning_only)) + self.with_col(|col| { + col.get_queued_cards(input.fetch_limit as usize, input.intraday_learning_only) + }) } } diff --git a/rslib/src/lib.rs b/rslib/src/lib.rs index eb5711ba5..bdc34c752 100644 --- a/rslib/src/lib.rs +++ b/rslib/src/lib.rs @@ -41,3 +41,11 @@ pub mod timestamp; pub mod types; pub mod undo; pub mod version; + +use std::env; + +use lazy_static::lazy_static; + +lazy_static! { + pub(crate) static ref PYTHON_UNIT_TESTS: bool = env::var("ANKI_TEST_MODE").is_ok(); +} diff --git a/rslib/src/scheduler/answering/mod.rs b/rslib/src/scheduler/answering/mod.rs index 78a9f8d51..164fcd405 100644 --- a/rslib/src/scheduler/answering/mod.rs +++ b/rslib/src/scheduler/answering/mod.rs @@ -414,7 +414,7 @@ impl Collection { /// Return a consistent seed for a given card at a given number of reps. /// If in test environment, disable fuzzing. fn get_fuzz_seed(card: &Card) -> Option { - if *crate::timestamp::TESTING || cfg!(test) { + if *crate::PYTHON_UNIT_TESTS || cfg!(test) { None } else { Some((card.id.0 as u64).wrapping_add(card.reps as u64)) diff --git a/rslib/src/scheduler/queue/builder/mod.rs b/rslib/src/scheduler/queue/builder/mod.rs index 47b189db8..4504dced7 100644 --- a/rslib/src/scheduler/queue/builder/mod.rs +++ b/rslib/src/scheduler/queue/builder/mod.rs @@ -120,7 +120,8 @@ impl QueueBuilder { // intraday learning let learning = sort_learning(self.learning); - let cutoff = TimestampSecs::now().adding_secs(learn_ahead_secs); + let now = TimestampSecs::now(); + let cutoff = now.adding_secs(learn_ahead_secs); let learn_count = learning.iter().take_while(|e| e.due <= cutoff).count(); // merge interday learning into main, and cap to parent review count @@ -143,12 +144,12 @@ impl QueueBuilder { review: review_count, learning: learn_count, }, - undo: Vec::new(), main: main_iter.collect(), - learning, + intraday_learning: learning, learn_ahead_secs, selected_deck, current_day, + current_learning_cutoff: now, } } } diff --git a/rslib/src/scheduler/queue/entry.rs b/rslib/src/scheduler/queue/entry.rs index 1079505cb..796590ab4 100644 --- a/rslib/src/scheduler/queue/entry.rs +++ b/rslib/src/scheduler/queue/entry.rs @@ -78,3 +78,15 @@ impl From for QueueEntry { Self::Main(e) } } + +impl From<&LearningQueueEntry> for QueueEntry { + fn from(e: &LearningQueueEntry) -> Self { + Self::IntradayLearning(*e) + } +} + +impl From<&MainQueueEntry> for QueueEntry { + fn from(e: &MainQueueEntry) -> Self { + Self::Main(*e) + } +} diff --git a/rslib/src/scheduler/queue/learning.rs b/rslib/src/scheduler/queue/learning.rs index c74818eed..b2d6ba86d 100644 --- a/rslib/src/scheduler/queue/learning.rs +++ b/rslib/src/scheduler/queue/learning.rs @@ -3,7 +3,7 @@ use std::{cmp::Ordering, collections::VecDeque}; -use super::CardQueues; +use super::{undo::CutoffSnapshot, CardQueues}; use crate::{prelude::*, scheduler::timing::SchedTimingToday}; #[derive(Debug, Clone, Copy, PartialEq, PartialOrd, Eq, Ord)] @@ -15,36 +15,43 @@ pub(crate) struct LearningQueueEntry { } impl CardQueues { - /// Check for any newly due cards, and then return the first, if any, - /// that is due before now. - pub(super) fn next_learning_entry_due_before_now( - &mut self, - now: TimestampSecs, - ) -> Option { - let learn_ahead_cutoff = now.adding_secs(self.learn_ahead_secs); - self.check_for_newly_due_learning_cards(learn_ahead_cutoff); - self.next_learning_entry_learning_ahead() - .filter(|c| c.due <= now) + /// Intraday learning cards that can be shown immediately. + pub(super) fn intraday_now_iter(&self) -> impl Iterator { + let cutoff = self.current_learning_cutoff; + self.intraday_learning + .iter() + .take_while(move |e| e.due <= cutoff) } - /// Check for due learning cards up to the learn ahead limit. - /// Does not check for newly due cards, as that is already done by - /// next_learning_entry_due_before_now() - pub(super) fn next_learning_entry_learning_ahead(&self) -> Option { - self.learning.front().copied() + /// Intraday learning cards that can be shown after the main queue is empty. + pub(super) fn intraday_ahead_iter(&self) -> impl Iterator { + let cutoff = self.current_learning_cutoff; + let ahead_cutoff = self.current_learn_ahead_cutoff(); + self.intraday_learning + .iter() + .skip_while(move |e| e.due <= cutoff) + .take_while(move |e| e.due <= ahead_cutoff) } - pub(super) fn pop_learning_entry(&mut self, id: CardId) -> Option { - if let Some(top) = self.learning.front() { - if top.id == id { - // under normal circumstances this should not go below 0, but currently - // the Python unit tests answer learning cards before they're due - self.counts.learning = self.counts.learning.saturating_sub(1); - return self.learning.pop_front(); - } - } + /// Increase the cutoff to the current time, and increase the learning count + /// for any new cards that now fall within the cutoff. + pub(super) fn check_for_newly_due_intraday_learning(&mut self) -> Box { + let change = CutoffSnapshot { + learning_count: self.counts.learning, + learning_cutoff: self.current_learning_cutoff, + }; + let cutoff = self.current_learning_cutoff; + let ahead_cutoff = self.current_learn_ahead_cutoff(); + let new_learning_cards = self + .intraday_learning + .iter() + .skip(self.counts.learning) + .take_while(|e| e.due <= ahead_cutoff) + .count(); + self.counts.learning += new_learning_cards; + self.current_learning_cutoff = cutoff; - None + Box::new(change) } /// Given the just-answered `card`, place it back in the learning queues if it's still @@ -65,18 +72,31 @@ impl CardQueues { mtime: card.mtime, }; - Some(self.requeue_learning_entry(entry, timing)) + Some(self.requeue_learning_entry(entry)) + } + + pub(super) fn cutoff_snapshot(&self) -> Box { + Box::new(CutoffSnapshot { + learning_count: self.counts.learning, + learning_cutoff: self.current_learning_cutoff, + }) + } + + pub(super) fn restore_cutoff(&mut self, change: &CutoffSnapshot) -> Box { + let current = self.cutoff_snapshot(); + self.counts.learning = change.learning_count; + self.current_learning_cutoff = change.learning_cutoff; + current } /// Caller must have validated learning entry is due today. pub(super) fn requeue_learning_entry( &mut self, mut entry: LearningQueueEntry, - timing: SchedTimingToday, ) -> LearningQueueEntry { - let cutoff = timing.now.adding_secs(self.learn_ahead_secs); + let cutoff = self.current_learn_ahead_cutoff(); if entry.due <= cutoff && self.learning_collapsed() { - if let Some(next) = self.learning.front() { + if let Some(next) = self.intraday_learning.front() { // ensure the card is scheduled after the next due card if next.due >= entry.due { if next.due < cutoff { @@ -94,7 +114,7 @@ impl CardQueues { } } - self.push_learning_card(entry); + self.insert_intraday_learning_card(entry); entry } @@ -103,41 +123,60 @@ impl CardQueues { self.main.is_empty() } - /// Adds card, maintaining correct sort order, and increments learning count if - /// card is due within cutoff. - pub(super) fn push_learning_card(&mut self, entry: LearningQueueEntry) { - let target_idx = - binary_search_by(&self.learning, |e| e.due.cmp(&entry.due)).unwrap_or_else(|e| e); - if target_idx < self.counts.learning { + /// Remove the head of the intraday learning queue, and update counts. + pub(super) fn pop_intraday_learning(&mut self) -> Option { + self.intraday_learning.pop_front().map(|head| { + // FIXME: + // under normal circumstances this should not go below 0, but currently + // the Python unit tests answer learning cards before they're due + self.counts.learning = self.counts.learning.saturating_sub(1); + head + }) + } + + /// Add an undone entry to the top of the intraday learning queue. + pub(super) fn push_intraday_learning(&mut self, entry: LearningQueueEntry) { + self.intraday_learning.push_front(entry); + self.counts.learning += 1; + } + + /// Adds an intraday learning card to the correct position of the queue, and + /// increments learning count if card is due. + pub(super) fn insert_intraday_learning_card(&mut self, entry: LearningQueueEntry) { + if entry.due <= self.current_learn_ahead_cutoff() { self.counts.learning += 1; } - self.learning.insert(target_idx, entry); + + let target_idx = binary_search_by(&self.intraday_learning, |e| e.due.cmp(&entry.due)) + .unwrap_or_else(|e| e); + self.intraday_learning.insert(target_idx, entry); } - fn check_for_newly_due_learning_cards(&mut self, cutoff: TimestampSecs) { - self.counts.learning += self - .learning - .iter() - .skip(self.counts.learning) - .take_while(|e| e.due <= cutoff) - .count(); - } - - pub(super) fn remove_requeued_learning_card_after_undo(&mut self, id: CardId) { - let due_idx = self.learning.iter().enumerate().find_map(|(idx, entry)| { - if entry.id == id { - Some(idx) - } else { - None + /// Remove an inserted intraday learning card after a lapse is undone, adjusting + /// counts. + pub(super) fn remove_intraday_learning_card( + &mut self, + card_id: CardId, + ) -> Option { + if let Some(position) = self.intraday_learning.iter().position(|e| e.id == card_id) { + let entry = self.intraday_learning.remove(position).unwrap(); + if entry.due + <= self + .current_learning_cutoff + .adding_secs(self.learn_ahead_secs) + { + self.counts.learning -= 1; } - }); - if let Some(idx) = due_idx { - // FIXME: if we remove the saturating_sub from pop_learning(), maybe - // this can go too? - self.counts.learning = self.counts.learning.saturating_sub(1); - self.learning.remove(idx); + Some(entry) + } else { + None } } + + fn current_learn_ahead_cutoff(&self) -> TimestampSecs { + self.current_learning_cutoff + .adding_secs(self.learn_ahead_secs) + } } /// Adapted from the Rust stdlib VecDeque implementation; we can drop this when the following diff --git a/rslib/src/scheduler/queue/main.rs b/rslib/src/scheduler/queue/main.rs index c42cc60b6..ab0f6b1c9 100644 --- a/rslib/src/scheduler/queue/main.rs +++ b/rslib/src/scheduler/queue/main.rs @@ -18,21 +18,23 @@ pub(crate) enum MainQueueEntryKind { } impl CardQueues { - pub(super) fn next_main_entry(&self) -> Option { - self.main.front().copied() + /// Remove the head of the main queue, and update counts. + pub(super) fn pop_main(&mut self) -> Option { + self.main.pop_front().map(|head| { + match head.kind { + MainQueueEntryKind::New => self.counts.new -= 1, + MainQueueEntryKind::Review => self.counts.review -= 1, + }; + head + }) } - pub(super) fn pop_main_entry(&mut self, id: CardId) -> Option { - if let Some(last) = self.main.front() { - if last.id == id { - match last.kind { - MainQueueEntryKind::New => self.counts.new -= 1, - MainQueueEntryKind::Review => self.counts.review -= 1, - } - return self.main.pop_front(); - } - } - - None + /// Add an undone entry to the top of the main queue. + pub(super) fn push_main(&mut self, entry: MainQueueEntry) { + match entry.kind { + MainQueueEntryKind::New => self.counts.new += 1, + MainQueueEntryKind::Review => self.counts.review += 1, + }; + self.main.push_front(entry); } } diff --git a/rslib/src/scheduler/queue/mod.rs b/rslib/src/scheduler/queue/mod.rs index 3e5b0a7bc..2c3c2f879 100644 --- a/rslib/src/scheduler/queue/mod.rs +++ b/rslib/src/scheduler/queue/mod.rs @@ -22,13 +22,14 @@ use crate::{backend_proto as pb, prelude::*, timestamp::TimestampSecs}; #[derive(Debug)] pub(crate) struct CardQueues { counts: Counts, - /// Any undone items take precedence. - undo: Vec, main: VecDeque, - learning: VecDeque, + intraday_learning: VecDeque, selected_deck: DeckId, current_day: u32, learn_ahead_secs: i64, + /// Updated each time a card is answered. Ensures we don't show a newly-due + /// learning card after a user returns from editing a review card. + current_learning_cutoff: TimestampSecs, } #[derive(Debug, Copy, Clone)] @@ -53,26 +54,43 @@ pub(crate) struct QueuedCards { } impl CardQueues { - /// Get the next due card, if there is one. - fn next_entry(&mut self, now: TimestampSecs) -> Option { - self.next_undo_entry() + /// An iterator over the card queues, in the order the cards will + /// be presented. + fn iter(&self) -> impl Iterator + '_ { + self.intraday_now_iter() .map(Into::into) - .or_else(|| self.next_learning_entry_due_before_now(now).map(Into::into)) - .or_else(|| self.next_main_entry().map(Into::into)) - .or_else(|| self.next_learning_entry_learning_ahead().map(Into::into)) + .chain(self.main.iter().map(Into::into)) + .chain(self.intraday_ahead_iter().map(Into::into)) } - /// Remove the provided card from the top of the queues. - /// If it was not at the top, return an error. - fn pop_answered(&mut self, id: CardId) -> Result { - if let Some(entry) = self.pop_undo_entry(id) { - Ok(entry) - } else if let Some(entry) = self.pop_main_entry(id) { - Ok(entry.into()) - } else if let Some(entry) = self.pop_learning_entry(id) { - Ok(entry.into()) + /// Remove the provided card from the top of the queues and + /// adjust the counts. If it was not at the top, return an error. + fn pop_entry(&mut self, id: CardId) -> Result { + let mut entry = self.iter().next(); + if entry.is_none() && *crate::PYTHON_UNIT_TESTS { + // the existing Python tests answer learning cards + // before they're due; try the first non-due learning + // card as a backup + entry = self.intraday_learning.front().map(Into::into) + } + if let Some(entry) = entry { + match entry { + QueueEntry::IntradayLearning(e) if e.id == id => { + self.pop_intraday_learning().map(Into::into) + } + QueueEntry::Main(e) if e.id == id => self.pop_main().map(Into::into), + _ => None, + } + .ok_or_else(|| AnkiError::invalid_input("not at top of queue")) } else { - Err(AnkiError::invalid_input("not at top of queue")) + Err(AnkiError::invalid_input("queues are empty")) + } + } + + fn push_undo_entry(&mut self, entry: QueueEntry) { + match entry { + QueueEntry::IntradayLearning(entry) => self.push_intraday_learning(entry), + QueueEntry::Main(entry) => self.push_main(entry), } } @@ -83,26 +101,12 @@ impl CardQueues { fn is_stale(&self, current_day: u32) -> bool { self.current_day != current_day } - - fn update_after_answering_card( - &mut self, - card: &Card, - timing: SchedTimingToday, - ) -> Result> { - let entry = self.pop_answered(card.id)?; - let requeued_learning = self.maybe_requeue_learning_card(card, timing); - - Ok(Box::new(QueueUpdate { - entry, - learning_requeue: requeued_learning, - })) - } } impl Collection { pub(crate) fn get_queued_cards( &mut self, - fetch_limit: u32, + fetch_limit: usize, intraday_learning_only: bool, ) -> Result { if let Some(next_cards) = self.next_cards(fetch_limit, intraday_learning_only)? { @@ -139,25 +143,34 @@ impl Collection { timing: SchedTimingToday, ) -> Result<()> { if let Some(queues) = &mut self.state.card_queues { - let mutation = queues.update_after_answering_card(card, timing)?; - self.save_queue_update_undo(mutation); - Ok(()) + let entry = queues.pop_entry(card.id)?; + let requeued_learning = queues.maybe_requeue_learning_card(card, timing); + let cutoff_change = queues.check_for_newly_due_intraday_learning(); + self.save_queue_update_undo(Box::new(QueueUpdate { + entry, + learning_requeue: requeued_learning, + })); + self.save_cutoff_change(cutoff_change); } else { // we currenly allow the queues to be empty for unit tests - Ok(()) } + + Ok(()) } pub(crate) fn get_queues(&mut self) -> Result<&mut CardQueues> { let timing = self.timing_today()?; let deck = self.get_current_deck_id(); - let need_rebuild = self + let day_rolled_over = self .state .card_queues .as_ref() .map(|q| q.is_stale(timing.days_elapsed)) - .unwrap_or(true); - if need_rebuild { + .unwrap_or(false); + if day_rolled_over { + self.discard_undo_and_study_queues(); + } + if self.state.card_queues.is_none() { self.state.card_queues = Some(self.build_queues(deck)?); } @@ -166,36 +179,46 @@ impl Collection { fn next_cards( &mut self, - _fetch_limit: u32, - _intraday_learning_only: bool, + fetch_limit: usize, + intraday_learning_only: bool, ) -> Result> { let queues = self.get_queues()?; - let mut cards = vec![]; - if let Some(entry) = queues.next_entry(TimestampSecs::now()) { - let card = self - .storage - .get_card(entry.card_id())? - .ok_or(AnkiError::NotFound)?; - if card.mtime != entry.mtime() { - return Err(AnkiError::invalid_input( - "bug: card modified without updating queue", - )); - } - - // fixme: pass in card instead of id - let next_states = self.get_next_card_states(card.id)?; - - cards.push(QueuedCard { - card, - next_states, - kind: entry.kind(), - }); - } - - if cards.is_empty() { + let counts = queues.counts(); + let entries: Vec<_> = if intraday_learning_only { + queues + .intraday_now_iter() + .chain(queues.intraday_ahead_iter()) + .map(Into::into) + .collect() + } else { + queues.iter().take(fetch_limit).collect() + }; + if entries.is_empty() { Ok(None) } else { - let counts = self.get_queues()?.counts(); + let cards: Vec<_> = entries + .into_iter() + .map(|entry| { + let card = self + .storage + .get_card(entry.card_id())? + .ok_or(AnkiError::NotFound)?; + if card.mtime != entry.mtime() { + return Err(AnkiError::invalid_input( + "bug: card modified without updating queue", + )); + } + + // fixme: pass in card instead of id + let next_states = self.get_next_card_states(card.id)?; + + Ok(QueuedCard { + card, + next_states, + kind: entry.kind(), + }) + }) + .collect::>()?; Ok(Some(QueuedCards { cards, new_count: counts.new, @@ -215,7 +238,7 @@ impl Collection { .map(|mut resp| resp.cards.pop().unwrap())) } - pub(crate) fn get_queue_single(&mut self) -> Result { + fn get_queue_single(&mut self) -> Result { self.next_cards(1, false)?.ok_or(AnkiError::NotFound) } diff --git a/rslib/src/scheduler/queue/undo.rs b/rslib/src/scheduler/queue/undo.rs index f1bebdffc..8fe72e360 100644 --- a/rslib/src/scheduler/queue/undo.rs +++ b/rslib/src/scheduler/queue/undo.rs @@ -1,13 +1,14 @@ // Copyright: Ankitects Pty Ltd and contributors // License: GNU AGPL, version 3 or later; http://www.gnu.org/licenses/agpl.html -use super::{CardQueues, LearningQueueEntry, QueueEntry, QueueEntryKind}; +use super::{LearningQueueEntry, QueueEntry}; use crate::prelude::*; #[derive(Debug)] pub(crate) enum UndoableQueueChange { CardAnswered(Box), CardAnswerUndone(Box), + CutoffChange(Box), } #[derive(Debug)] @@ -16,13 +17,21 @@ pub(crate) struct QueueUpdate { pub learning_requeue: Option, } +/// Stores the old learning count and cutoff prior to the +/// cutoff being adjusted after answering a card. +#[derive(Debug)] +pub(crate) struct CutoffSnapshot { + pub learning_count: usize, + pub learning_cutoff: TimestampSecs, +} + impl Collection { pub(crate) fn undo_queue_change(&mut self, change: UndoableQueueChange) -> Result<()> { match change { UndoableQueueChange::CardAnswered(update) => { let queues = self.get_queues()?; if let Some(learning) = &update.learning_requeue { - queues.remove_requeued_learning_card_after_undo(learning.id); + queues.remove_intraday_learning_card(learning.id); } queues.push_undo_entry(update.entry); self.save_undo(UndoableQueueChange::CardAnswerUndone(update)); @@ -30,12 +39,20 @@ impl Collection { Ok(()) } UndoableQueueChange::CardAnswerUndone(update) => { - // don't try to update existing queue when redoing; just - // rebuild it instead - self.clear_study_queues(); - // but preserve undo state for a subsequent undo + let queues = self.get_queues()?; + if let Some(learning) = update.learning_requeue { + queues.insert_intraday_learning_card(learning); + } + queues.pop_entry(update.entry.card_id())?; self.save_undo(UndoableQueueChange::CardAnswered(update)); + Ok(()) + } + UndoableQueueChange::CutoffChange(change) => { + let queues = self.get_queues()?; + let current = queues.restore_cutoff(&change); + self.save_cutoff_change(current); + Ok(()) } } @@ -44,37 +61,9 @@ impl Collection { pub(super) fn save_queue_update_undo(&mut self, change: Box) { self.save_undo(UndoableQueueChange::CardAnswered(change)) } -} -impl CardQueues { - pub(super) fn next_undo_entry(&self) -> Option { - self.undo.last().copied() - } - - pub(super) fn pop_undo_entry(&mut self, id: CardId) -> Option { - if let Some(last) = self.undo.last() { - if last.card_id() == id { - match last.kind() { - QueueEntryKind::New => self.counts.new -= 1, - QueueEntryKind::Review => self.counts.review -= 1, - QueueEntryKind::Learning => self.counts.learning -= 1, - } - return self.undo.pop(); - } - } - - None - } - - /// Add an undone card back to the 'front' of the list, and update - /// the counts. - pub(super) fn push_undo_entry(&mut self, entry: QueueEntry) { - match entry.kind() { - QueueEntryKind::New => self.counts.new += 1, - QueueEntryKind::Review => self.counts.review += 1, - QueueEntryKind::Learning => self.counts.learning += 1, - } - self.undo.push(entry); + pub(super) fn save_cutoff_change(&mut self, change: Box) { + self.save_undo(UndoableQueueChange::CutoffChange(change)) } } @@ -188,11 +177,8 @@ mod test { let deck = col.get_deck(DeckId(1))?.unwrap(); assert_eq!(deck.common.review_studied, 0); - assert_eq!(col.next_card()?.is_some(), true); - - let q = col.get_queue_single()?; - assert_eq!(&[q.new_count, q.learning_count, q.review_count], &[0, 0, 1]); + assert_eq!(col.counts(), [0, 0, 1]); Ok(()) }; @@ -212,19 +198,23 @@ mod test { } #[test] - #[ignore = "undo code is currently broken"] - fn undo_counts1() -> Result<()> { + fn undo_counts() -> Result<()> { let mut col = open_test_collection(); assert_eq!(col.counts(), [0, 0, 0]); add_note(&mut col, true)?; assert_eq!(col.counts(), [2, 0, 0]); - col.answer_good(); + col.answer_again(); assert_eq!(col.counts(), [1, 1, 0]); col.answer_good(); assert_eq!(col.counts(), [0, 2, 0]); + col.answer_again(); + assert_eq!(col.counts(), [0, 2, 0]); + // first card graduates col.answer_good(); assert_eq!(col.counts(), [0, 1, 0]); + // other card is all that is left, so the + // last step is deferred col.answer_good(); assert_eq!(col.counts(), [0, 0, 0]); @@ -234,57 +224,26 @@ mod test { col.undo()?; assert_eq!(col.counts(), [0, 2, 0]); col.undo()?; + assert_eq!(col.counts(), [0, 2, 0]); + col.undo()?; assert_eq!(col.counts(), [1, 1, 0]); col.undo()?; assert_eq!(col.counts(), [2, 0, 0]); col.undo()?; assert_eq!(col.counts(), [0, 0, 0]); - Ok(()) - } - - #[test] - fn undo_counts2() -> Result<()> { - let mut col = open_test_collection(); - - assert_eq!(col.counts(), [0, 0, 0]); - add_note(&mut col, true)?; + // and forwards again + col.redo()?; assert_eq!(col.counts(), [2, 0, 0]); - col.answer_again(); + col.redo()?; assert_eq!(col.counts(), [1, 1, 0]); - col.answer_again(); + col.redo()?; assert_eq!(col.counts(), [0, 2, 0]); - col.answer_again(); + col.redo()?; assert_eq!(col.counts(), [0, 2, 0]); - col.answer_again(); - assert_eq!(col.counts(), [0, 2, 0]); - col.answer_good(); - assert_eq!(col.counts(), [0, 2, 0]); - col.answer_easy(); + col.redo()?; assert_eq!(col.counts(), [0, 1, 0]); - col.answer_good(); - // last card, due in a minute - assert_eq!(col.counts(), [0, 0, 0]); - - Ok(()) - } - - #[test] - fn undo_counts_relearn() -> Result<()> { - let mut col = open_test_collection(); - - add_note(&mut col, true)?; - col.storage - .db - .execute_batch("update cards set due=0,queue=2,type=2")?; - assert_eq!(col.counts(), [0, 0, 2]); - col.answer_again(); - assert_eq!(col.counts(), [0, 1, 1]); - col.answer_again(); - assert_eq!(col.counts(), [0, 2, 0]); - col.answer_easy(); - assert_eq!(col.counts(), [0, 1, 0]); - col.answer_easy(); + col.redo()?; assert_eq!(col.counts(), [0, 0, 0]); Ok(()) diff --git a/rslib/src/timestamp.rs b/rslib/src/timestamp.rs index e37f395a0..5a790ea35 100644 --- a/rslib/src/timestamp.rs +++ b/rslib/src/timestamp.rs @@ -1,10 +1,9 @@ // Copyright: Ankitects Pty Ltd and contributors // License: GNU AGPL, version 3 or later; http://www.gnu.org/licenses/agpl.html -use std::{env, time}; +use std::time; use chrono::prelude::*; -use lazy_static::lazy_static; use crate::define_newtype; @@ -69,12 +68,8 @@ impl TimestampMillis { } } -lazy_static! { - pub(crate) static ref TESTING: bool = env::var("ANKI_TEST_MODE").is_ok(); -} - fn elapsed() -> time::Duration { - if *TESTING { + if *crate::PYTHON_UNIT_TESTS { // shift clock around rollover time to accomodate Python tests that make bad assumptions. // we should update the tests in the future and remove this hack. let mut elap = time::SystemTime::now() diff --git a/rslib/src/undo/mod.rs b/rslib/src/undo/mod.rs index 5f79e3ae6..f54a43c39 100644 --- a/rslib/src/undo/mod.rs +++ b/rslib/src/undo/mod.rs @@ -84,7 +84,6 @@ impl UndoManager { } fn begin_step(&mut self, op: Option) { - println!("begin: {:?}", op); if op.is_none() { self.undo_steps.clear(); self.redo_steps.clear(); @@ -116,7 +115,6 @@ impl UndoManager { println!("no undo changes, discarding step"); } } - println!("ended, undo steps count now {}", self.undo_steps.len()); } fn can_undo(&self) -> Option<&Op> {