Skip to main content

max / goingson

Cover the weekly review orchestrator and the search clamp compute_weekly_review had no direct test: the counts, carry-over window, vacation passthrough and event mapping were only exercised through the command layer. Cover it with and without a stored review, and fill in parse_week_start, the project-health status bands, and focused-project grouping while in there. The search limit/offset clamp was likewise untested, and SQLite reads LIMIT -1 as unbounded, so a negative limit would have dumped the table. Also correct CONTRIBUTING: the JS test runner exists and passes.
Co-Authored-By
Claude Opus 5 (1M context) <noreply@anthropic.com>
Author: Max Johnson <me@maxj.phd> · 2026-07-26 14:04 UTC
Signed with PGP, not checked
Commit: 51f9b60f931c51803860c833f6ac6396fb8d82d7
Parent: d39e0fd
3 files changed, +304 insertions, -1 deletion
M CONTRIBUTING.md +5 -1
@@ -268,7 +268,11 @@
268 268
269 269 - **Rust unit tests:** In-file `#[cfg(test)]` modules in each crate
270 270 - **Rust integration tests:** `tests/` directories in each crate
271 - - **JS tests:** Manual testing via the app (no automated JS test runner yet)
271 + - **JS tests:** `node src-tauri/frontend/js/tests/run.js` — no npm, plain Node.
272 + `run.js` mocks the browser globals, `require`s the modules under test in
273 + `index.html` order, then runs the suites. Adding a module to the suite means
274 + adding its `require` plus whatever browser API it touches to the mock block.
275 + Modules that need a real DOM are still covered by hand in the app.
272 276 - Test databases use in-memory SQLite (`:memory:`) with migrations applied
273 277 - Always verify the full pipeline: Rust command → repository → DB → response → JS render
274 278
M crates/core/src/weekly_review.rs +263
@@ -783,4 +783,267 @@
783 783 assert_eq!(days[5].events.len(), 0, "Saturday should have 0 events");
784 784 assert_eq!(days[6].events.len(), 0, "Sunday should have 0 events");
785 785 }
786 +
787 + /// Creates a minimal task with the given status. `completed_at` only
788 + /// survives for Completed, matching what the repositories return.
789 + fn make_task(status: TaskStatus, created_at: DateTime<Utc>) -> Task {
790 + let mut task = make_completed_task(created_at, created_at);
791 + task.status = status;
792 + if task.status != TaskStatus::Completed {
793 + task.completed_at = None;
794 + }
795 + task
796 + }
797 +
798 + fn make_project(name: &str) -> crate::models::Project {
799 + crate::models::Project {
800 + id: ProjectId::new(),
801 + name: name.to_string(),
802 + description: String::new(),
803 + project_type: crate::models::ProjectType::SideProject,
804 + status: crate::models::ProjectStatus::Active,
805 + created_at: Utc::now(),
806 + group_id: None,
807 + }
808 + }
809 +
810 + fn utc(y: i32, m: u32, d: u32) -> DateTime<Utc> {
811 + NaiveDate::from_ymd_opt(y, m, d)
812 + .unwrap()
813 + .and_hms_opt(12, 0, 0)
814 + .map(|dt| DateTime::<Utc>::from_naive_utc_and_offset(dt, Utc))
815 + .unwrap()
816 + }
817 +
818 + #[test]
819 + fn test_parse_week_start_snaps_to_monday() {
820 + let monday = NaiveDate::from_ymd_opt(2026, 2, 9).unwrap();
821 + // Any day in the week snaps back to its Monday.
822 + assert_eq!(parse_week_start("2026-02-09"), Some(monday));
823 + assert_eq!(parse_week_start("2026-02-12"), Some(monday));
824 + assert_eq!(parse_week_start("2026-02-15"), Some(monday)); // Sunday
825 + // The next Monday is its own week, not the previous one.
826 + assert_eq!(
827 + parse_week_start("2026-02-16"),
828 + NaiveDate::from_ymd_opt(2026, 2, 16)
829 + );
830 + assert_eq!(parse_week_start("not-a-date"), None);
831 + assert_eq!(parse_week_start("2026-02-30"), None);
832 + }
833 +
834 + #[test]
835 + fn test_compute_project_health_status_bands() {
836 + let healthy = make_project("healthy");
837 + let warning = make_project("warning");
838 + let danger = make_project("danger");
839 + let untouched = make_project("untouched");
840 +
841 + let past = Utc::now() - Duration::days(1);
842 + let mut tasks = vec![];
843 +
844 + // healthy: one pending, nothing overdue.
845 + let mut t = make_task(TaskStatus::Pending, utc(2026, 2, 2));
846 + t.project_id = Some(healthy.id);
847 + tasks.push(t);
848 +
849 + // warning: one overdue out of two.
850 + for overdue in [true, false] {
851 + let mut t = make_task(TaskStatus::Started, utc(2026, 2, 2));
852 + t.project_id = Some(warning.id);
853 + if overdue {
854 + t.due = Some(past);
855 + }
856 + tasks.push(t);
857 + }
858 +
859 + // danger: three overdue, plus a deleted task that must not be counted.
860 + for _ in 0..3 {
861 + let mut t = make_task(TaskStatus::Pending, utc(2026, 2, 2));
862 + t.project_id = Some(danger.id);
863 + t.due = Some(past);
864 + tasks.push(t);
865 + }
866 + let mut deleted = make_task(TaskStatus::Deleted, utc(2026, 2, 2));
867 + deleted.project_id = Some(danger.id);
868 + deleted.due = Some(past);
869 + tasks.push(deleted);
870 +
871 + let health = compute_project_health(
872 + &[
873 + healthy.clone(),
874 + warning.clone(),
875 + danger.clone(),
876 + untouched.clone(),
877 + ],
878 + &tasks,
879 + );
880 +
881 + // A project with no tasks is dropped entirely, not reported as healthy.
882 + assert_eq!(health.len(), 3);
883 + assert!(health.iter().all(|h| h.id != untouched.id));
884 +
885 + let by_name = |name: &str| health.iter().find(|h| h.name == name).unwrap();
886 + let h = by_name("healthy");
887 + assert_eq!((h.active_count, h.overdue_count, h.total_count), (1, 0, 1));
888 + assert_eq!(h.status, "healthy");
889 +
890 + let w = by_name("warning");
891 + assert_eq!((w.active_count, w.overdue_count, w.total_count), (2, 1, 2));
892 + assert_eq!(w.status, "warning");
893 +
894 + let d = by_name("danger");
895 + assert_eq!(
896 + (d.active_count, d.overdue_count, d.total_count),
897 + (3, 3, 3),
898 + "the deleted task must be excluded from every count"
899 + );
900 + assert_eq!(d.status, "danger");
901 + }
902 +
903 + #[test]
904 + fn test_compute_focused_projects_groups_by_project() {
905 + let alpha = ProjectId::new();
906 + let beta = ProjectId::new();
907 +
908 + let mut a1 = make_task(TaskStatus::Pending, utc(2026, 2, 9));
909 + a1.project_id = Some(alpha);
910 + a1.project_name = Some("Alpha".to_string());
911 + let mut a2 = make_task(TaskStatus::Started, utc(2026, 2, 9));
912 + a2.project_id = Some(alpha);
913 + a2.project_name = Some("Alpha".to_string());
914 + // No project_name from the query: falls back to "Unknown".
915 + let mut b1 = make_task(TaskStatus::Pending, utc(2026, 2, 9));
916 + b1.project_id = Some(beta);
917 + b1.project_name = None;
918 + // Projectless focus tasks contribute nothing.
919 + let loose = make_task(TaskStatus::Pending, utc(2026, 2, 9));
920 +
921 + let projects = compute_focused_projects(&[a1, a2, b1, loose]);
922 + assert_eq!(projects.len(), 2);
923 +
924 + let a = projects.iter().find(|p| p.id == alpha).unwrap();
925 + assert_eq!(a.name, "Alpha");
926 + assert_eq!(a.focused_task_count, 2);
927 +
928 + let b = projects.iter().find(|p| p.id == beta).unwrap();
929 + assert_eq!(b.name, "Unknown");
930 + assert_eq!(b.focused_task_count, 1);
931 + }
932 +
933 + #[test]
934 + fn test_compute_weekly_review_counts_and_carryover() {
935 + let week_start = NaiveDate::from_ymd_opt(2026, 2, 9).unwrap(); // Monday
936 + let project = make_project("Alpha");
937 +
938 + // Pending work older than the week is carry-over; pending work created
939 + // inside the week is not.
940 + let mut carried = make_task(TaskStatus::Pending, utc(2026, 1, 20));
941 + carried.project_id = Some(project.id);
942 + let fresh = make_task(TaskStatus::Started, utc(2026, 2, 10));
943 + // Completed and deleted tasks never carry over.
944 + let done = make_task(TaskStatus::Completed, utc(2026, 1, 20));
945 + let deleted = make_task(TaskStatus::Deleted, utc(2026, 1, 20));
946 +
947 + let mut focused = make_task(TaskStatus::Pending, utc(2026, 2, 10));
948 + focused.project_id = Some(project.id);
949 + focused.project_name = Some("Alpha".to_string());
950 + focused.is_focus = true;
951 +
952 + let occurred = make_event("standup", utc(2026, 2, 10));
953 +
954 + let review = WeeklyReview {
955 + id: WeeklyReviewId::new(),
956 + user_id: UserId::new(),
957 + week_start_date: week_start,
958 + notes: "shipped the installer".to_string(),
959 + completed_at: utc(2026, 2, 15),
960 + vacation_days: vec![5, 6],
961 + };
962 +
963 + let data = compute_weekly_review(WeeklyReviewInput {
964 + week_start,
965 + review: Some(review.clone()),
966 + tasks_completed: vec![done.clone()],
967 + tasks_overdue: vec![carried.clone()],
968 + events_occurred: vec![occurred],
969 + upcoming_events: vec![],
970 + tasks_due_next_week: vec![fresh.clone()],
971 + tasks_already_overdue: vec![carried.clone()],
972 + all_tasks: vec![carried.clone(), fresh.clone(), done, deleted],
973 + focused_tasks: vec![focused],
974 + available_for_focus: vec![],
975 + projects: vec![project.clone()],
976 + tz: Tz::UTC,
977 + });
978 +
979 + assert_eq!(data.week_start_date, "2026-02-09");
980 + assert_eq!(data.week_end_date, "2026-02-15");
981 + assert_eq!(data.week_display, "Feb 09 - Feb 15");
982 +
983 + // Review fields come straight off the stored row.
984 + assert!(data.is_completed);
985 + assert_eq!(data.completed_at, Some(review.completed_at));
986 + assert_eq!(data.notes, "shipped the installer");
987 + assert_eq!(data.vacation_days, vec![5, 6]);
988 + assert!(!data.show_nudge, "a completed review never nudges");
989 +
990 + // Counts mirror the pre-fetched lists.
991 + assert_eq!(data.tasks_completed_count, 1);
992 + assert_eq!(data.tasks_overdue_count, 1);
993 + assert_eq!(data.tasks_due_next_week_count, 1);
994 + assert_eq!(data.tasks_already_overdue_count, 1);
995 + assert_eq!(data.events_occurred_count, 1);
996 + assert_eq!(data.events_occurred.len(), 1);
997 + assert_eq!(data.events_occurred[0].title, "standup");
998 + assert_eq!(data.events_occurred[0].formatted_time, "Tue 12:00");
999 +
1000 + // Pending = Pending + Started; carry-over is the pre-week subset.
1001 + assert_eq!(data.tasks_pending_count, 2);
1002 + assert_eq!(data.carried_over_count, 1);
1003 + assert_eq!(data.carried_over_tasks.len(), 1);
1004 + assert_eq!(data.carried_over_tasks[0].id, carried.id);
1005 +
1006 + assert_eq!(data.timeline_days.len(), 7);
1007 + assert!(data.timeline_days[5].is_vacation);
1008 + assert!(data.timeline_days[6].is_vacation);
1009 + assert!(!data.timeline_days[0].is_vacation);
1010 +
1011 + // Derived from the focus list, not fetched.
1012 + assert_eq!(data.focused_projects.len(), 1);
1013 + assert_eq!(data.focused_projects[0].name, "Alpha");
1014 + assert_eq!(data.focused_projects[0].focused_task_count, 1);
1015 +
1016 + assert_eq!(data.project_health.len(), 1);
1017 + assert_eq!(data.project_health[0].name, "Alpha");
1018 + }
1019 +
1020 + #[test]
1021 + fn test_compute_weekly_review_without_stored_review() {
1022 + let week_start = NaiveDate::from_ymd_opt(2026, 2, 9).unwrap();
1023 + let data = compute_weekly_review(WeeklyReviewInput {
1024 + week_start,
1025 + review: None,
1026 + tasks_completed: vec![],
1027 + tasks_overdue: vec![],
1028 + events_occurred: vec![],
1029 + upcoming_events: vec![],
1030 + tasks_due_next_week: vec![],
1031 + tasks_already_overdue: vec![],
1032 + all_tasks: vec![],
1033 + focused_tasks: vec![],
1034 + available_for_focus: vec![],
1035 + projects: vec![],
1036 + tz: Tz::UTC,
1037 + });
1038 +
1039 + assert!(!data.is_completed);
1040 + assert_eq!(data.completed_at, None);
1041 + assert_eq!(data.notes, "");
1042 + assert!(data.vacation_days.is_empty());
1043 + assert!(data.timeline_days.iter().all(|d| !d.is_vacation));
1044 + assert_eq!(data.tasks_pending_count, 0);
1045 + assert_eq!(data.carried_over_count, 0);
1046 + assert!(data.project_health.is_empty());
1047 + assert!(data.focused_projects.is_empty());
1048 + }
786 1049 }
M crates/db-sqlite/tests/search_repo_tests.rs +36
@@ -313,6 +313,42 @@
313 313 assert_eq!(offset.len(), all.len() - 2);
314 314 }
315 315
316 + #[tokio::test]
317 + async fn search_negative_limit_and_offset_are_clamped() {
318 + let pool = common::setup_test_db().await;
319 + let user_id = common::create_test_user(&pool).await;
320 +
321 + for i in 1..=5 {
322 + create_task_with_desc(&pool, user_id, &format!("Clamped task number {i}")).await;
323 + }
324 +
325 + let repo = SqliteSearchRepository::new(pool);
326 +
327 + // SQLite reads `LIMIT -1` as unbounded, so a negative limit must clamp to
328 + // 0 rows rather than dumping the whole table past the caller's page size.
329 + let mut negative_limit = SearchQuery::new("clamped");
330 + negative_limit.limit = Some(-1);
331 + let (results, _total) = repo.search(user_id, negative_limit).await.unwrap();
332 + assert!(
333 + results.is_empty(),
334 + "a negative limit must not return unbounded rows"
335 + );
336 +
337 + // A negative offset behaves as offset 0, not as a reversed window.
338 + let mut negative_offset = SearchQuery::new("clamped");
339 + negative_offset.offset = Some(-3);
340 + negative_offset.limit = Some(50);
341 + let (results, _total) = repo.search(user_id, negative_offset).await.unwrap();
342 + assert_eq!(results.len(), 5);
343 +
344 + // offset + limit is a saturating add, so an extreme pair cannot overflow.
345 + let mut huge = SearchQuery::new("clamped");
346 + huge.offset = Some(i64::MAX);
347 + huge.limit = Some(i64::MAX);
348 + let results = repo.search(user_id, huge).await;
349 + assert!(results.is_ok(), "offset + limit must not overflow");
350 + }
351 +
316 352 // Edge Cases
317 353
318 354 #[tokio::test]