fix(screen_locker): parse exercises from JSON column, show reason in suspicious message

- Rewrite _get_today_exercise_count() to parse JSON from workouts.exercises
  column instead of broken JOIN on exercises definition table
- Show actual reason (stale/no_exercises) instead of generic 'suspicious'
- Fix pylint issues: generated-members regex for mock assertions, design
  limits for mixins/tests, concurrent.futures no-name-in-module disable,
  implicit booleanness in assertions, module-level pylint disables in tests
- Add pytest to pre-commit pylint additional_dependencies
- Add tests for missing exercises column, null/malformed JSON, nameless
  exercise entries
This commit is contained in:
Krzysztof kuhy Rudnicki 2026-04-10 18:11:30 +02:00
parent 1322700cc8
commit 7f2d2c4c39
6 changed files with 130 additions and 62 deletions

View File

@ -155,6 +155,7 @@ repos:
- --fail-under=8.0 - --fail-under=8.0
- --jobs=0 - --jobs=0
additional_dependencies: additional_dependencies:
- pytest
- python-chess - python-chess
- requests - requests
- pygame - pygame

View File

@ -180,18 +180,31 @@ enable = "all"
disable = [] disable = []
[tool.pylint.design] [tool.pylint.design]
# A class with just run() as public API is valid for games/apps # Mixins and single-entry-point classes may have zero public methods
min-public-methods = 1 min-public-methods = 0
# Enforce maximum file length of 500 lines # Test modules can be large
max-module-lines = 500 max-module-lines = 1000
# UI/mixin classes accumulate attributes across multiple mixins
max-attributes = 10
[tool.pylint.spelling] [tool.pylint.spelling]
# No spelling dictionary to avoid false positives # No spelling dictionary to avoid false positives
spelling-dict = "" spelling-dict = ""
[tool.pylint.typecheck] [tool.pylint.typecheck]
# cv2 (OpenCV) dynamically loads members from C extension at runtime # cv2 (OpenCV) dynamically loads members from C extension at runtime.
generated-members = ["cv2.*"] # unittest.mock.MagicMock generates assertion/introspection methods at runtime.
generated-members = [
"cv2.*",
".*\\.assert_called_once_with",
".*\\.assert_called_once",
".*\\.assert_called",
".*\\.assert_not_called",
".*\\.assert_any_call",
".*\\.call_args",
".*\\.call_args_list",
".*\\.call_count",
]
# ============================================================================ # ============================================================================
# BANDIT - Security linter # BANDIT - Security linter

View File

@ -2,8 +2,12 @@
from __future__ import annotations from __future__ import annotations
from concurrent.futures import ThreadPoolExecutor, as_completed from concurrent.futures import ( # pylint: disable=no-name-in-module
ThreadPoolExecutor,
as_completed,
)
import contextlib import contextlib
import json
import logging import logging
from pathlib import Path from pathlib import Path
import shutil import shutil
@ -52,7 +56,7 @@ class PhoneVerificationMixin:
except subprocess.TimeoutExpired: except subprocess.TimeoutExpired:
_logger.warning("ADB command timed out: %s", args) _logger.warning("ADB command timed out: %s", args)
return False, "" return False, ""
return result.returncode == 0, result.stdout return not result.returncode, result.stdout
def _adb_shell( def _adb_shell(
self, self,
@ -216,31 +220,37 @@ class PhoneVerificationMixin:
def _get_today_exercise_count(self, db_path: Path) -> int: def _get_today_exercise_count(self, db_path: Path) -> int:
"""Count distinct exercises in today's workouts. """Count distinct exercises in today's workouts.
Uses the StrongLifts 'exercises' table joined with 'workouts' to Parses the JSON ``exercises`` column in the ``workouts`` table.
verify that actual exercises were logged, not just empty sessions. Each workout row stores its exercises as a JSON array, not in a
separate relational table.
Args: Args:
db_path: Path to the locally-pulled StrongLifts database. db_path: Path to the locally-pulled StrongLifts database.
Returns: Returns:
Number of distinct exercises in today's workouts. Number of distinct exercises across today's workouts.
Returns 0 on any error. Returns 0 on any error.
""" """
try: try:
conn = sqlite3.connect(str(db_path)) conn = sqlite3.connect(str(db_path))
try: try:
cursor = conn.execute( cursor = conn.execute(
"SELECT COUNT(DISTINCT e.exercise) " "SELECT exercises FROM workouts "
"FROM exercises e " "WHERE date(start / 1000, 'unixepoch', 'localtime') "
"JOIN workouts w ON e.workout = w.id "
"WHERE date(w.start / 1000, 'unixepoch', 'localtime') "
"= date('now', 'localtime')", "= date('now', 'localtime')",
) )
row = cursor.fetchone() exercise_ids: set[str] = set()
return int(row[0]) if row else 0 for (exercises_json,) in cursor:
if not exercises_json:
continue
for ex in json.loads(exercises_json):
ex_id = ex.get("id") or ex.get("name", "")
if ex_id:
exercise_ids.add(ex_id)
return len(exercise_ids)
finally: finally:
conn.close() conn.close()
except (sqlite3.Error, ValueError, TypeError): except (sqlite3.Error, ValueError, TypeError, json.JSONDecodeError):
_logger.warning("Failed to query exercise count") _logger.warning("Failed to query exercise count")
return 0 return 0

View File

@ -2,7 +2,7 @@
from __future__ import annotations from __future__ import annotations
from concurrent.futures import ThreadPoolExecutor from concurrent.futures import ThreadPoolExecutor # pylint: disable=no-name-in-module
from typing import TYPE_CHECKING from typing import TYPE_CHECKING
from python_pkg.screen_locker._constants import ( from python_pkg.screen_locker._constants import (
@ -79,9 +79,7 @@ class UIFlowsMixin:
) )
elif status in ("stale", "no_exercises"): elif status in ("stale", "no_exercises"):
self._show_retry_and_sick( self._show_retry_and_sick(
f"\u274c {message}\n\n" f"\u274c {message}\n\nReason: {status}",
"The workout data looks suspicious.\n"
"Make sure you did a real workout today.",
) )
elif status == "clock_tampered": elif status == "clock_tampered":
self._show_retry_and_sick( self._show_retry_and_sick(

View File

@ -1,7 +1,9 @@
"""Tests for ADB commands, phone connection, and database operations.""" """Tests for ADB commands, phone connection, and database operations."""
# pylint: disable=protected-access,unused-argument
from __future__ import annotations from __future__ import annotations
import json
import sqlite3 import sqlite3
import subprocess import subprocess
import time import time
@ -71,7 +73,7 @@ class TestRunAdb:
success, output = locker._run_adb(["devices"]) success, output = locker._run_adb(["devices"])
assert success is False assert success is False
assert output == "" assert not output
def test_run_adb_oserror( def test_run_adb_oserror(
self, self,
@ -88,7 +90,7 @@ class TestRunAdb:
success, output = locker._run_adb(["devices"]) success, output = locker._run_adb(["devices"])
assert success is False assert success is False
assert output == "" assert not output
def test_run_adb_timeout( def test_run_adb_timeout(
self, self,
@ -105,7 +107,7 @@ class TestRunAdb:
success, output = locker._run_adb(["devices"]) success, output = locker._run_adb(["devices"])
assert success is False assert success is False
assert output == "" assert not output
class TestAdbShell: class TestAdbShell:
@ -417,7 +419,7 @@ class TestCountTodayWorkouts:
conn.commit() conn.commit()
conn.close() conn.close()
assert locker._count_today_workouts(db_file) == 0 assert not locker._count_today_workouts(db_file)
def test_invalid_db_returns_zero( def test_invalid_db_returns_zero(
self, self,
@ -430,7 +432,7 @@ class TestCountTodayWorkouts:
bad_file = tmp_path / "not_a_db.db" bad_file = tmp_path / "not_a_db.db"
bad_file.write_text("not a database") bad_file.write_text("not a database")
assert locker._count_today_workouts(bad_file) == 0 assert not locker._count_today_workouts(bad_file)
def test_missing_table_returns_zero( def test_missing_table_returns_zero(
self, self,
@ -446,7 +448,7 @@ class TestCountTodayWorkouts:
conn.commit() conn.commit()
conn.close() conn.close()
assert locker._count_today_workouts(db_file) == 0 assert not locker._count_today_workouts(db_file)
def test_multiple_workouts_today( def test_multiple_workouts_today(
self, self,
@ -528,7 +530,7 @@ class TestGetTodayWorkoutDurationMinutes:
conn.commit() conn.commit()
conn.close() conn.close()
assert locker._get_today_workout_duration_minutes(db_file) == 0.0 assert not locker._get_today_workout_duration_minutes(db_file)
def test_sums_multiple_workouts( def test_sums_multiple_workouts(
self, self,
@ -583,7 +585,7 @@ class TestGetTodayWorkoutDurationMinutes:
conn.commit() conn.commit()
conn.close() conn.close()
assert locker._get_today_workout_duration_minutes(db_file) == 0.0 assert not locker._get_today_workout_duration_minutes(db_file)
def test_invalid_db_returns_zero( def test_invalid_db_returns_zero(
self, self,
@ -596,7 +598,7 @@ class TestGetTodayWorkoutDurationMinutes:
bad_file = tmp_path / "not_a_db.db" bad_file = tmp_path / "not_a_db.db"
bad_file.write_text("not a database") bad_file.write_text("not a database")
assert locker._get_today_workout_duration_minutes(bad_file) == 0.0 assert not locker._get_today_workout_duration_minutes(bad_file)
def test_missing_table_returns_zero( def test_missing_table_returns_zero(
self, self,
@ -612,7 +614,7 @@ class TestGetTodayWorkoutDurationMinutes:
conn.commit() conn.commit()
conn.close() conn.close()
assert locker._get_today_workout_duration_minutes(db_file) == 0.0 assert not locker._get_today_workout_duration_minutes(db_file)
class TestGetTodayExerciseCount: class TestGetTodayExerciseCount:
@ -630,27 +632,20 @@ class TestGetTodayExerciseCount:
conn = sqlite3.connect(str(db_file)) conn = sqlite3.connect(str(db_file))
conn.execute( conn.execute(
"CREATE TABLE workouts " "CREATE TABLE workouts "
"(id TEXT PRIMARY KEY, start INTEGER, finish INTEGER)", "(id TEXT PRIMARY KEY, start INTEGER, finish INTEGER, exercises TEXT)",
)
conn.execute(
"CREATE TABLE exercises (id TEXT, workout TEXT, exercise TEXT)",
) )
now_ms = int(time.time() * 1000) now_ms = int(time.time() * 1000)
conn.execute( exercises_json = json.dumps(
"INSERT INTO workouts VALUES (?, ?, ?)", [
("w1", now_ms, now_ms + 3600000), {"id": "squat", "name": "Squat"},
{"id": "bench_press", "name": "Bench Press"},
{"id": "squat", "name": "Squat"},
{"category": "WARMUP"},
]
) )
conn.execute( conn.execute(
"INSERT INTO exercises VALUES (?, ?, ?)", "INSERT INTO workouts VALUES (?, ?, ?, ?)",
("e1", "w1", "squat"), ("w1", now_ms, now_ms + 3600000, exercises_json),
)
conn.execute(
"INSERT INTO exercises VALUES (?, ?, ?)",
("e2", "w1", "bench_press"),
)
conn.execute(
"INSERT INTO exercises VALUES (?, ?, ?)",
("e3", "w1", "squat"),
) )
conn.commit() conn.commit()
conn.close() conn.close()
@ -669,20 +664,17 @@ class TestGetTodayExerciseCount:
conn = sqlite3.connect(str(db_file)) conn = sqlite3.connect(str(db_file))
conn.execute( conn.execute(
"CREATE TABLE workouts " "CREATE TABLE workouts "
"(id TEXT PRIMARY KEY, start INTEGER, finish INTEGER)", "(id TEXT PRIMARY KEY, start INTEGER, finish INTEGER, exercises TEXT)",
)
conn.execute(
"CREATE TABLE exercises (id TEXT, workout TEXT, exercise TEXT)",
) )
now_ms = int(time.time() * 1000) now_ms = int(time.time() * 1000)
conn.execute( conn.execute(
"INSERT INTO workouts VALUES (?, ?, ?)", "INSERT INTO workouts VALUES (?, ?, ?, ?)",
("w1", now_ms, now_ms + 3600000), ("w1", now_ms, now_ms + 3600000, "[]"),
) )
conn.commit() conn.commit()
conn.close() conn.close()
assert locker._get_today_exercise_count(db_file) == 0 assert not locker._get_today_exercise_count(db_file)
def test_invalid_db_returns_zero( def test_invalid_db_returns_zero(
self, self,
@ -695,15 +687,15 @@ class TestGetTodayExerciseCount:
bad_file = tmp_path / "bad.db" bad_file = tmp_path / "bad.db"
bad_file.write_text("not a db") bad_file.write_text("not a db")
assert locker._get_today_exercise_count(bad_file) == 0 assert not locker._get_today_exercise_count(bad_file)
def test_missing_table_returns_zero_exercises( def test_missing_exercises_column_returns_zero(
self, self,
mock_tk: MagicMock, mock_tk: MagicMock,
mock_sys_exit: MagicMock, mock_sys_exit: MagicMock,
tmp_path: Path, tmp_path: Path,
) -> None: ) -> None:
"""Test returns 0 when exercises table doesn't exist.""" """Test returns 0 when workouts table has no exercises column."""
locker = create_locker(mock_tk, tmp_path) locker = create_locker(mock_tk, tmp_path)
db_file = tmp_path / "empty.db" db_file = tmp_path / "empty.db"
conn = sqlite3.connect(str(db_file)) conn = sqlite3.connect(str(db_file))
@ -711,10 +703,63 @@ class TestGetTodayExerciseCount:
"CREATE TABLE workouts " "CREATE TABLE workouts "
"(id TEXT PRIMARY KEY, start INTEGER, finish INTEGER)", "(id TEXT PRIMARY KEY, start INTEGER, finish INTEGER)",
) )
now_ms = int(time.time() * 1000)
conn.execute(
"INSERT INTO workouts VALUES (?, ?, ?)",
("w1", now_ms, now_ms + 3600000),
)
conn.commit() conn.commit()
conn.close() conn.close()
assert locker._get_today_exercise_count(db_file) == 0 assert not locker._get_today_exercise_count(db_file)
def test_null_exercises_json_returns_zero(
self,
mock_tk: MagicMock,
mock_sys_exit: MagicMock,
tmp_path: Path,
) -> None:
"""Test returns 0 when exercises JSON is NULL."""
locker = create_locker(mock_tk, tmp_path)
db_file = tmp_path / "null_ex.db"
conn = sqlite3.connect(str(db_file))
conn.execute(
"CREATE TABLE workouts "
"(id TEXT PRIMARY KEY, start INTEGER, finish INTEGER, exercises TEXT)",
)
now_ms = int(time.time() * 1000)
conn.execute(
"INSERT INTO workouts VALUES (?, ?, ?, ?)",
("w1", now_ms, now_ms + 3600000, None),
)
conn.commit()
conn.close()
assert not locker._get_today_exercise_count(db_file)
def test_malformed_exercises_json_returns_zero(
self,
mock_tk: MagicMock,
mock_sys_exit: MagicMock,
tmp_path: Path,
) -> None:
"""Test returns 0 when exercises JSON is malformed."""
locker = create_locker(mock_tk, tmp_path)
db_file = tmp_path / "bad_json.db"
conn = sqlite3.connect(str(db_file))
conn.execute(
"CREATE TABLE workouts "
"(id TEXT PRIMARY KEY, start INTEGER, finish INTEGER, exercises TEXT)",
)
now_ms = int(time.time() * 1000)
conn.execute(
"INSERT INTO workouts VALUES (?, ?, ?, ?)",
("w1", now_ms, now_ms + 3600000, "not valid json"),
)
conn.commit()
conn.close()
assert not locker._get_today_exercise_count(db_file)
class TestIsWorkoutFinishRecent: class TestIsWorkoutFinishRecent:

View File

@ -1,4 +1,5 @@
"""Tests for phone workout verification, phone check, and unlock operations.""" """Tests for phone workout verification, phone check, and unlock operations."""
# pylint: disable=protected-access,unused-argument
from __future__ import annotations from __future__ import annotations
@ -390,7 +391,7 @@ class TestStartPhoneCheck:
locker._show_retry_and_sick.assert_called_once() locker._show_retry_and_sick.assert_called_once()
call_args = locker._show_retry_and_sick.call_args[0][0] call_args = locker._show_retry_and_sick.call_args[0][0]
assert "suspicious" in call_args.lower() assert "reason: stale" in call_args.lower()
def test_handle_startup_no_exercises_shows_retry_and_sick( def test_handle_startup_no_exercises_shows_retry_and_sick(
self, self,
@ -405,7 +406,7 @@ class TestStartPhoneCheck:
locker._show_retry_and_sick.assert_called_once() locker._show_retry_and_sick.assert_called_once()
call_args = locker._show_retry_and_sick.call_args[0][0] call_args = locker._show_retry_and_sick.call_args[0][0]
assert "suspicious" in call_args.lower() assert "reason: no_exercises" in call_args.lower()
def test_handle_startup_clock_tampered_shows_retry_and_sick( def test_handle_startup_clock_tampered_shows_retry_and_sick(
self, self,