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

@@ -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):

rslib/src/backend/dbproxy.rs

@@ -111,7 +111,7 @@ pub(super) fn db_command_bytes(col: &mut Collection, input: &[u8]) -> Result<Vec
 
 fn update_state_after_modification(col: &mut Collection, sql: &str) {
     if !is_dql(sql) {
-        println!("clearing undo+study due to {}", sql);
+        // println!("clearing undo+study due to {}", sql);
         col.update_state_after_dbproxy_modification();
     }
 }

rslib/src/backend/scheduler/mod.rs

@@ -177,7 +177,9 @@ impl SchedulingService for Backend {
     }
 
     fn get_queued_cards(&self, input: pb::GetQueuedCardsIn) -> Result<pb::GetQueuedCardsOut> {
-        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)
+        })
     }
 }
 

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();
+}

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<u64> {
-    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))

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,
         }
     }
 }

rslib/src/scheduler/queue/entry.rs

@@ -78,3 +78,15 @@ impl From<MainQueueEntry> 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)
+    }
+}

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<LearningQueueEntry> {
-        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<Item = &LearningQueueEntry> {
+        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<LearningQueueEntry> {
-        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<Item = &LearningQueueEntry> {
+        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<LearningQueueEntry> {
-        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<CutoffSnapshot> {
+        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<CutoffSnapshot> {
+        Box::new(CutoffSnapshot {
+            learning_count: self.counts.learning,
+            learning_cutoff: self.current_learning_cutoff,
+        })
+    }
+
+    pub(super) fn restore_cutoff(&mut self, change: &CutoffSnapshot) -> Box<CutoffSnapshot> {
+        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<LearningQueueEntry> {
+        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<LearningQueueEntry> {
+        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

rslib/src/scheduler/queue/main.rs

@@ -18,21 +18,23 @@ pub(crate) enum MainQueueEntryKind {
 }
 
 impl CardQueues {
-    pub(super) fn next_main_entry(&self) -> Option<MainQueueEntry> {
-        self.main.front().copied()
+    /// Remove the head of the main queue, and update counts.
+    pub(super) fn pop_main(&mut self) -> Option<MainQueueEntry> {
+        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<MainQueueEntry> {
-        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);
     }
 }

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<QueueEntry>,
     main: VecDeque<MainQueueEntry>,
-    learning: VecDeque<LearningQueueEntry>,
+    intraday_learning: VecDeque<LearningQueueEntry>,
     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<QueueEntry> {
-        self.next_undo_entry()
+    /// An iterator over the card queues, in the order the cards will
+    /// be presented.
+    fn iter(&self) -> impl Iterator<Item = QueueEntry> + '_ {
+        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<QueueEntry> {
-        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<QueueEntry> {
+        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<Box<QueueUpdate>> {
-        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<pb::GetQueuedCardsOut> {
         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<Option<QueuedCards>> {
         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::<Result<_>>()?;
             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<QueuedCards> {
+    fn get_queue_single(&mut self) -> Result<QueuedCards> {
         self.next_cards(1, false)?.ok_or(AnkiError::NotFound)
     }
 

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<QueueUpdate>),
     CardAnswerUndone(Box<QueueUpdate>),
+    CutoffChange(Box<CutoffSnapshot>),
 }
 
 #[derive(Debug)]
@@ -16,13 +17,21 @@ pub(crate) struct QueueUpdate {
     pub learning_requeue: Option<LearningQueueEntry>,
 }
 
+/// 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<QueueUpdate>) {
         self.save_undo(UndoableQueueChange::CardAnswered(change))
     }
-}
 
-impl CardQueues {
-    pub(super) fn next_undo_entry(&self) -> Option<QueueEntry> {
-        self.undo.last().copied()
-    }
-
-    pub(super) fn pop_undo_entry(&mut self, id: CardId) -> Option<QueueEntry> {
-        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<CutoffSnapshot>) {
+        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(())

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()

rslib/src/undo/mod.rs

@@ -84,7 +84,6 @@ impl UndoManager {
     }
 
     fn begin_step(&mut self, op: Option<Op>) {
-        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> {