mirror of
https://github.com/ChuckBuilds/LEDMatrix.git
synced 2026-08-18 00:58:08 +00:00
Compare commits
2
Commits
| Author | SHA1 | Date | |
|---|---|---|---|
|
9083df9f5c | ||
|
|
0901d044d3 |
@@ -24,9 +24,29 @@ echo "========================================"
|
|||||||
# Auto-detect latest version if needed
|
# Auto-detect latest version if needed
|
||||||
if [ "$PIXLET_VERSION" = "latest" ]; then
|
if [ "$PIXLET_VERSION" = "latest" ]; then
|
||||||
echo "Detecting latest version..."
|
echo "Detecting latest version..."
|
||||||
PIXLET_VERSION=$(curl -s "https://api.github.com/repos/${REPO}/releases/latest" | grep '"tag_name"' | sed -E 's/.*"([^"]+)".*/\1/')
|
# When this response arrives on a single line -- as it did on the device
|
||||||
if [ -z "$PIXLET_VERSION" ]; then
|
# where Starlark apps were failing -- `grep '"tag_name"'` matches the whole
|
||||||
echo "Failed to detect latest version, using fallback"
|
# document and a greedy `sed 's/.*"([^"]+)".*/\1/'` captures the LAST
|
||||||
|
# quoted token in it rather than the tag. That resolved to
|
||||||
|
# "mentions_count", which built a download URL for a release that does not
|
||||||
|
# exist. (The API is pretty-printed by default, which is why the old
|
||||||
|
# command looks correct when you try it by hand -- but the formatting is
|
||||||
|
# not something to depend on.) Match the field itself and take the value
|
||||||
|
# after it, which is right for either shape.
|
||||||
|
PIXLET_VERSION=$(curl -fsSL "https://api.github.com/repos/${REPO}/releases/latest" \
|
||||||
|
| grep -o '"tag_name"[[:space:]]*:[[:space:]]*"[^"]*"' \
|
||||||
|
| head -n1 \
|
||||||
|
| sed -E 's/.*:[[:space:]]*"([^"]*)".*/\1/')
|
||||||
|
|
||||||
|
# A wrong-but-non-empty value is what made the old bug silent, so check the
|
||||||
|
# shape rather than just that something came back. Anchored at both ends: a
|
||||||
|
# partial match would accept "v0.53garbage" or "0.53" and build a URL for a
|
||||||
|
# release that cannot exist, which is the failure this check is here to
|
||||||
|
# stop. Every tronbyt/pixlet release to date is vX.Y.Z; the optional suffix
|
||||||
|
# leaves room for a future -rc.1 or +build tag.
|
||||||
|
if ! printf '%s' "$PIXLET_VERSION" \
|
||||||
|
| grep -qE '^v[0-9]+\.[0-9]+\.[0-9]+([-+][0-9A-Za-z.-]+)?$'; then
|
||||||
|
echo "Could not detect the latest version (got: '${PIXLET_VERSION:-<empty>}'), using fallback"
|
||||||
PIXLET_VERSION="v0.50.2"
|
PIXLET_VERSION="v0.50.2"
|
||||||
fi
|
fi
|
||||||
fi
|
fi
|
||||||
@@ -67,8 +87,26 @@ download_binary() {
|
|||||||
temp_dir=$(mktemp -d -p "$PROJECT_ROOT" -t pixlet_download.XXXXXXXXXX)
|
temp_dir=$(mktemp -d -p "$PROJECT_ROOT" -t pixlet_download.XXXXXXXXXX)
|
||||||
local temp_file="$temp_dir/$archive_name"
|
local temp_file="$temp_dir/$archive_name"
|
||||||
|
|
||||||
if ! curl -L -o "$temp_file" "$url" 2>/dev/null; then
|
# -f so an HTTP error is a failure. Without it curl writes the 404 body
|
||||||
echo "✗ Failed to download $arch"
|
# to the file and exits 0, and the first sign of trouble is tar saying
|
||||||
|
# "not in gzip format" about what is actually a page of HTML.
|
||||||
|
if ! curl -fL -o "$temp_file" "$url" 2>/dev/null; then
|
||||||
|
echo "✗ Failed to download $arch from $url"
|
||||||
|
rm -rf "$temp_dir"
|
||||||
|
return 1
|
||||||
|
fi
|
||||||
|
|
||||||
|
# Belt and braces: a mirror or proxy can return 200 with an error page.
|
||||||
|
if ! gzip -t "$temp_file" 2>/dev/null; then
|
||||||
|
echo "✗ Downloaded file is not a gzip archive: $url"
|
||||||
|
# These bytes come from whatever answered the request, so strip
|
||||||
|
# everything non-printable before echoing them: an error page carrying
|
||||||
|
# terminal escapes would otherwise be able to rewrite this output or
|
||||||
|
# bury it in a CI log. Printable characters are kept rather than
|
||||||
|
# hex-encoding the lot, because "<!DOCTYPE html>" is the diagnostic.
|
||||||
|
local first_bytes
|
||||||
|
first_bytes=$(head -c 60 "$temp_file" | tr -cd '[:print:]')
|
||||||
|
printf ' (first bytes: %s)\n' "$first_bytes"
|
||||||
rm -rf "$temp_dir"
|
rm -rf "$temp_dir"
|
||||||
return 1
|
return 1
|
||||||
fi
|
fi
|
||||||
|
|||||||
@@ -125,6 +125,14 @@ class PluginManager:
|
|||||||
self._plugin_locks: Dict[str, threading.Lock] = {}
|
self._plugin_locks: Dict[str, threading.Lock] = {}
|
||||||
self._plugin_locks_guard = threading.Lock()
|
self._plugin_locks_guard = threading.Lock()
|
||||||
self._update_worker: Optional[threading.Thread] = None
|
self._update_worker: Optional[threading.Thread] = None
|
||||||
|
# Plugin ids whose update() has finished since the last time anyone
|
||||||
|
# asked. Updates are dispatched to a worker thread, so a caller that
|
||||||
|
# wants to know "whose data just changed" cannot learn it by diffing
|
||||||
|
# plugin_last_update around run_scheduled_updates() -- that call only
|
||||||
|
# enqueues, and the timestamp is stamped later, on the worker. See
|
||||||
|
# run_scheduled_updates_with_changes().
|
||||||
|
self._completed_updates: set = set()
|
||||||
|
self._completed_updates_lock = threading.Lock()
|
||||||
self._synchronous_updates = False
|
self._synchronous_updates = False
|
||||||
if self.config_manager is not None:
|
if self.config_manager is not None:
|
||||||
try:
|
try:
|
||||||
@@ -1025,6 +1033,7 @@ class PluginManager:
|
|||||||
if success:
|
if success:
|
||||||
with self._plugin_last_update_lock:
|
with self._plugin_last_update_lock:
|
||||||
self.plugin_last_update[plugin_id] = scheduled_time
|
self.plugin_last_update[plugin_id] = scheduled_time
|
||||||
|
self._note_update_completed(plugin_id)
|
||||||
self.state_manager.record_update(plugin_id)
|
self.state_manager.record_update(plugin_id)
|
||||||
self.state_manager.set_state(plugin_id, PluginState.ENABLED)
|
self.state_manager.set_state(plugin_id, PluginState.ENABLED)
|
||||||
if self.health_tracker:
|
if self.health_tracker:
|
||||||
@@ -1089,28 +1098,41 @@ class PluginManager:
|
|||||||
|
|
||||||
def run_scheduled_updates_with_changes(self, current_time: Optional[float] = None) -> List[str]:
|
def run_scheduled_updates_with_changes(self, current_time: Optional[float] = None) -> List[str]:
|
||||||
"""
|
"""
|
||||||
Like run_scheduled_updates(), but also returns the plugin_ids whose
|
Like run_scheduled_updates(), but also reports which plugins have
|
||||||
plugin_last_update timestamp actually advanced during this call.
|
fresh data -- the ids whose update() has finished since the last
|
||||||
|
call, not necessarily the ones enqueued by this one.
|
||||||
|
|
||||||
The before/after snapshots and the update pass itself are each
|
That distinction is the whole point. This used to snapshot
|
||||||
individually lock-protected against concurrent plugin_last_update
|
plugin_last_update, call run_scheduled_updates(), and diff. But
|
||||||
mutation (Vegas mode calls this from its own background
|
run_scheduled_updates() only *enqueues*: the work runs on the
|
||||||
update-tick thread, racing the main render loop's plugin updates),
|
update worker and the timestamp is stamped there, after this method
|
||||||
so callers get an atomic "who got fresh data" answer without
|
has already returned. The two snapshots were therefore always
|
||||||
reaching into plugin_last_update themselves. The lock is not held
|
identical and the result was always empty, so Vegas never learned
|
||||||
across the update pass so slow/blocking plugin update() calls don't
|
that any plugin's data had changed and kept scrolling whatever a
|
||||||
serialize against other plugin_last_update readers.
|
segment was first built from -- last night's live game still drawn
|
||||||
|
as live the next morning. The only path that ever worked was the
|
||||||
|
synchronous kill-switch, where update() runs inline.
|
||||||
|
|
||||||
|
Reporting completions instead of enqueues costs a poll's worth of
|
||||||
|
latency (the Vegas tick runs every ~4s) and is correct regardless of
|
||||||
|
which side of the queue the work lands on.
|
||||||
"""
|
"""
|
||||||
with self._plugin_last_update_lock:
|
|
||||||
old_times = dict(self.plugin_last_update)
|
|
||||||
|
|
||||||
self.run_scheduled_updates(current_time)
|
self.run_scheduled_updates(current_time)
|
||||||
|
return self.drain_completed_updates()
|
||||||
|
|
||||||
with self._plugin_last_update_lock:
|
def _note_update_completed(self, plugin_id: str) -> None:
|
||||||
return [
|
"""Record that a plugin's update() finished, for the next poll."""
|
||||||
plugin_id for plugin_id, new_time in self.plugin_last_update.items()
|
with self._completed_updates_lock:
|
||||||
if new_time > old_times.get(plugin_id, 0.0)
|
self._completed_updates.add(plugin_id)
|
||||||
]
|
|
||||||
|
def drain_completed_updates(self) -> List[str]:
|
||||||
|
"""Return and clear the plugin ids whose update() has since finished."""
|
||||||
|
with self._completed_updates_lock:
|
||||||
|
if not self._completed_updates:
|
||||||
|
return []
|
||||||
|
done = sorted(self._completed_updates)
|
||||||
|
self._completed_updates.clear()
|
||||||
|
return done
|
||||||
|
|
||||||
def update_all_plugins(self) -> None:
|
def update_all_plugins(self) -> None:
|
||||||
"""
|
"""
|
||||||
@@ -1135,6 +1157,7 @@ class PluginManager:
|
|||||||
if success:
|
if success:
|
||||||
with self._plugin_last_update_lock:
|
with self._plugin_last_update_lock:
|
||||||
self.plugin_last_update[plugin_id] = time.time()
|
self.plugin_last_update[plugin_id] = time.time()
|
||||||
|
self._note_update_completed(plugin_id)
|
||||||
self.state_manager.record_update(plugin_id)
|
self.state_manager.record_update(plugin_id)
|
||||||
self.state_manager.set_state(plugin_id, PluginState.ENABLED)
|
self.state_manager.set_state(plugin_id, PluginState.ENABLED)
|
||||||
else:
|
else:
|
||||||
|
|||||||
@@ -0,0 +1,180 @@
|
|||||||
|
"""
|
||||||
|
Tests for scripts/download_pixlet.sh -- release-tag resolution and download guards.
|
||||||
|
|
||||||
|
Background: Starlark apps render through the pixlet binary, and the installer
|
||||||
|
that fetches it failed silently. It resolved the release tag by grepping the
|
||||||
|
GitHub API response for '"tag_name"' and taking the last quoted token on the
|
||||||
|
match with a greedy sed. When the response arrives on one line that token is
|
||||||
|
"mentions_count", not the tag, so the script built a URL for a release that
|
||||||
|
cannot exist -- and `curl -L -o` without -f wrote the 404 body to the file and
|
||||||
|
exited 0, so the first sign of trouble was tar reporting "not in gzip format"
|
||||||
|
about a page of HTML.
|
||||||
|
|
||||||
|
The API is pretty-printed by default, which is exactly why this needs a test:
|
||||||
|
by hand the old command looks correct, and the failure only appears when the
|
||||||
|
formatting changes. These drive the real script with a stubbed curl on PATH, so
|
||||||
|
both response shapes are covered without touching the network.
|
||||||
|
"""
|
||||||
|
|
||||||
|
import re
|
||||||
|
import shutil
|
||||||
|
import subprocess
|
||||||
|
from pathlib import Path
|
||||||
|
|
||||||
|
import pytest
|
||||||
|
|
||||||
|
SCRIPT = Path(__file__).resolve().parent.parent / "scripts" / "download_pixlet.sh"
|
||||||
|
|
||||||
|
PRETTY = """{
|
||||||
|
"url": "https://api.github.com/repos/tronbyt/pixlet/releases/12345",
|
||||||
|
"id": 12345,
|
||||||
|
"tag_name": "v0.53.1",
|
||||||
|
"name": "v0.53.1",
|
||||||
|
"draft": false,
|
||||||
|
"prerelease": false,
|
||||||
|
"mentions_count": 3
|
||||||
|
}
|
||||||
|
"""
|
||||||
|
|
||||||
|
# The shape that broke it: one line, and the last quoted token is not the tag.
|
||||||
|
MINIFIED = (
|
||||||
|
'{"url":"https://api.github.com/repos/tronbyt/pixlet/releases/12345",'
|
||||||
|
'"id":12345,"tag_name":"v0.53.1","name":"v0.53.1","draft":false,'
|
||||||
|
'"prerelease":false,"mentions_count":3}'
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
|
def run_script(tmp_path, api_body, download=None):
|
||||||
|
"""Run the real script against a stubbed curl.
|
||||||
|
|
||||||
|
Args:
|
||||||
|
api_body: what the stub returns for the api.github.com request.
|
||||||
|
download: bytes to write for a release-asset request, or None to make
|
||||||
|
that request fail the way `curl -f` does on an HTTP error.
|
||||||
|
"""
|
||||||
|
root = tmp_path / "project"
|
||||||
|
(root / "scripts").mkdir(parents=True)
|
||||||
|
shutil.copy(SCRIPT, root / "scripts" / "download_pixlet.sh")
|
||||||
|
|
||||||
|
api_file = tmp_path / "api.json"
|
||||||
|
api_file.write_text(api_body)
|
||||||
|
|
||||||
|
stub_dir = tmp_path / "stub"
|
||||||
|
stub_dir.mkdir()
|
||||||
|
asset_file = tmp_path / "asset.bin"
|
||||||
|
if download is not None:
|
||||||
|
asset_file.write_bytes(download)
|
||||||
|
|
||||||
|
# Stands in for curl, including the -f semantics the fix turns on: without
|
||||||
|
# -f, real curl writes the error body to the output file and exits 0, which
|
||||||
|
# is what let a 404 masquerade as a successful download. The stub has to
|
||||||
|
# honour that or a test of the fix would pass against the old script too.
|
||||||
|
(stub_dir / "curl").write_text(f"""#!/bin/bash
|
||||||
|
out=""
|
||||||
|
url=""
|
||||||
|
fail_on_error=0
|
||||||
|
while [ $# -gt 0 ]; do
|
||||||
|
case "$1" in
|
||||||
|
-o) out="$2"; shift 2 ;;
|
||||||
|
-*f*) fail_on_error=1; shift ;;
|
||||||
|
-*) shift ;;
|
||||||
|
*) url="$1"; shift ;;
|
||||||
|
esac
|
||||||
|
done
|
||||||
|
if [[ "$url" == *api.github.com* ]]; then
|
||||||
|
cat {api_file}
|
||||||
|
exit 0
|
||||||
|
fi
|
||||||
|
if [ -f "{asset_file}" ]; then
|
||||||
|
cp "{asset_file}" "$out"
|
||||||
|
exit 0
|
||||||
|
fi
|
||||||
|
# No asset: stand in for an HTTP 404.
|
||||||
|
if [ "$fail_on_error" = "1" ]; then
|
||||||
|
exit 22
|
||||||
|
fi
|
||||||
|
printf '<!DOCTYPE html><html>404 Not Found</html>' > "$out"
|
||||||
|
exit 0
|
||||||
|
""")
|
||||||
|
(stub_dir / "curl").chmod(0o755)
|
||||||
|
|
||||||
|
return subprocess.run(
|
||||||
|
["bash", str(root / "scripts" / "download_pixlet.sh")],
|
||||||
|
capture_output=True, text=True,
|
||||||
|
env={"PATH": f"{stub_dir}:/usr/bin:/bin:/usr/sbin:/sbin",
|
||||||
|
"PIXLET_VERSION": "latest"},
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
|
def resolved_version(result):
|
||||||
|
match = re.search(r"^Version: (.+)$", result.stdout, re.M)
|
||||||
|
assert match, f"no version line in output:\n{result.stdout}"
|
||||||
|
return match.group(1).strip()
|
||||||
|
|
||||||
|
|
||||||
|
def test_script_is_syntactically_valid():
|
||||||
|
result = subprocess.run(["bash", "-n", str(SCRIPT)], capture_output=True, text=True)
|
||||||
|
assert result.returncode == 0, result.stderr
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.parametrize("body,label", [(PRETTY, "pretty"), (MINIFIED, "minified")])
|
||||||
|
def test_tag_is_resolved_from_either_response_shape(tmp_path, body, label):
|
||||||
|
"""The minified case is the regression: the last quoted token there is
|
||||||
|
"mentions_count", which is what the old greedy sed captured."""
|
||||||
|
result = run_script(tmp_path, body)
|
||||||
|
assert resolved_version(result) == "v0.53.1", f"{label}: {result.stdout}"
|
||||||
|
assert "mentions_count" not in result.stdout
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.parametrize(
|
||||||
|
"tag",
|
||||||
|
["mentions_count", "v0.53garbage", "0.53", "v0.5", "", "v0.53.1 ; echo pwned"],
|
||||||
|
)
|
||||||
|
def test_a_tag_that_is_not_a_release_falls_back(tmp_path, tag):
|
||||||
|
"""A wrong-but-non-empty value is what made the original bug silent, so the
|
||||||
|
check is on the shape. Partial matches must not pass: "v0.53garbage" and
|
||||||
|
"0.53" would build a URL for a release that cannot exist."""
|
||||||
|
result = run_script(tmp_path, '{"tag_name": "%s"}' % tag)
|
||||||
|
assert resolved_version(result) == "v0.50.2", result.stdout
|
||||||
|
assert "using fallback" in result.stdout
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.parametrize("tag", ["v0.53.1", "v1.0.0", "v0.54.0-rc.1", "v1.2.3+build.4"])
|
||||||
|
def test_real_release_tag_shapes_are_accepted(tmp_path, tag):
|
||||||
|
assert resolved_version(run_script(tmp_path, '{"tag_name": "%s"}' % tag)) == tag
|
||||||
|
|
||||||
|
|
||||||
|
def test_an_http_error_is_reported_as_a_failed_download(tmp_path):
|
||||||
|
"""Without curl -f the 404 body lands in the file and curl exits 0, so the
|
||||||
|
failure surfaced two steps later as tar complaining about gzip -- about
|
||||||
|
what was really a page of HTML. It has to be reported where it happened.
|
||||||
|
|
||||||
|
Both versions end at 0/1, so asserting only on the count would pass against
|
||||||
|
the old script; the discriminating part is which layer reports it.
|
||||||
|
"""
|
||||||
|
result = run_script(tmp_path, PRETTY, download=None)
|
||||||
|
assert "Download complete: 0/1 succeeded" in result.stdout
|
||||||
|
assert "✓ Downloaded" not in result.stdout
|
||||||
|
assert "Failed to download" in result.stdout
|
||||||
|
assert "Failed to extract" not in result.stdout, (
|
||||||
|
"an HTTP error should not surface as an extraction failure")
|
||||||
|
|
||||||
|
|
||||||
|
def test_a_non_archive_response_is_rejected_before_extraction(tmp_path):
|
||||||
|
result = run_script(tmp_path, PRETTY, download=b"<!DOCTYPE html><html>502 Bad Gateway")
|
||||||
|
assert "not a gzip archive" in result.stdout
|
||||||
|
assert "Download complete: 0/1 succeeded" in result.stdout
|
||||||
|
|
||||||
|
|
||||||
|
def test_the_diagnostic_cannot_smuggle_terminal_escapes(tmp_path):
|
||||||
|
"""Those bytes come from whatever answered the request. An error page
|
||||||
|
carrying escapes must not be able to rewrite the output or bury it."""
|
||||||
|
hostile = b"<!DOCTYPE html>\x1b[2J\x1b[31mgone\x1b[0m\rHTTP 200 OK\x08\x08"
|
||||||
|
result = run_script(tmp_path, PRETTY, download=hostile)
|
||||||
|
assert "not a gzip archive" in result.stdout
|
||||||
|
printed = re.search(r"^\s*\(first bytes: (.*)\)$", result.stdout, re.M)
|
||||||
|
assert printed, f"no diagnostic line:\n{result.stdout}"
|
||||||
|
assert "DOCTYPE" in printed.group(1), "the useful part of the page was dropped"
|
||||||
|
for forbidden in ("\x1b", "\r", "\x08", "\x00"):
|
||||||
|
assert forbidden not in printed.group(1), (
|
||||||
|
f"control byte {forbidden!r} reached the terminal")
|
||||||
@@ -0,0 +1,182 @@
|
|||||||
|
#!/usr/bin/env python3
|
||||||
|
"""
|
||||||
|
Tests that "which plugins have fresh data" survives the async update worker.
|
||||||
|
|
||||||
|
Regression under test: run_scheduled_updates_with_changes() snapshotted
|
||||||
|
plugin_last_update, called run_scheduled_updates(), and diffed the two. But
|
||||||
|
run_scheduled_updates() only *enqueues* -- the work runs on the update worker
|
||||||
|
and stamps the timestamp there, after the method has already returned. The
|
||||||
|
snapshots were therefore always identical and the result always empty.
|
||||||
|
|
||||||
|
Vegas depends on that result: it is what calls mark_plugin_updated(), which
|
||||||
|
drops the cached content for a plugin whose data changed. With it always
|
||||||
|
empty, a segment kept scrolling whatever it was first built from -- the
|
||||||
|
"last night's live game still drawn as live the next morning" failure the
|
||||||
|
coordinator comments describe. Observed on a live rig: zero update ticks in
|
||||||
|
twenty minutes, with weather, stocks and news all updating.
|
||||||
|
|
||||||
|
Run: python -m pytest test/test_update_change_reporting.py -v
|
||||||
|
"""
|
||||||
|
|
||||||
|
import ast
|
||||||
|
import inspect
|
||||||
|
import sys
|
||||||
|
import threading
|
||||||
|
import unittest
|
||||||
|
from pathlib import Path
|
||||||
|
|
||||||
|
sys.path.insert(0, str(Path(__file__).resolve().parent.parent))
|
||||||
|
|
||||||
|
from src.plugin_system.plugin_manager import PluginManager # noqa: E402
|
||||||
|
|
||||||
|
|
||||||
|
def _manager():
|
||||||
|
"""A PluginManager with only the update-reporting state initialised."""
|
||||||
|
manager = PluginManager.__new__(PluginManager)
|
||||||
|
manager._completed_updates = set()
|
||||||
|
manager._completed_updates_lock = threading.Lock()
|
||||||
|
return manager
|
||||||
|
|
||||||
|
|
||||||
|
class DrainCompletedUpdates(unittest.TestCase):
|
||||||
|
def setUp(self):
|
||||||
|
self.manager = _manager()
|
||||||
|
|
||||||
|
def test_nothing_completed_reports_nothing(self):
|
||||||
|
self.assertEqual(self.manager.drain_completed_updates(), [])
|
||||||
|
|
||||||
|
def test_a_completed_update_is_reported(self):
|
||||||
|
self.manager._note_update_completed("news")
|
||||||
|
self.assertEqual(self.manager.drain_completed_updates(), ["news"])
|
||||||
|
|
||||||
|
def test_draining_clears_so_the_next_poll_is_empty(self):
|
||||||
|
self.manager._note_update_completed("news")
|
||||||
|
self.manager.drain_completed_updates()
|
||||||
|
self.assertEqual(
|
||||||
|
self.manager.drain_completed_updates(), [],
|
||||||
|
"a plugin must be reported once per update, not on every poll, "
|
||||||
|
"or Vegas would drop its cached content every few seconds")
|
||||||
|
|
||||||
|
def test_repeated_completions_between_polls_collapse(self):
|
||||||
|
for _ in range(5):
|
||||||
|
self.manager._note_update_completed("weather")
|
||||||
|
self.assertEqual(self.manager.drain_completed_updates(), ["weather"])
|
||||||
|
|
||||||
|
def test_multiple_plugins_are_all_reported(self):
|
||||||
|
for plugin_id in ("news", "weather", "ledmatrix-stocks"):
|
||||||
|
self.manager._note_update_completed(plugin_id)
|
||||||
|
self.assertEqual(self.manager.drain_completed_updates(),
|
||||||
|
["ledmatrix-stocks", "news", "weather"])
|
||||||
|
|
||||||
|
|
||||||
|
class CompletionReportingIsAsyncSafe(unittest.TestCase):
|
||||||
|
"""The point of the change: completion may land after the call returns."""
|
||||||
|
|
||||||
|
def setUp(self):
|
||||||
|
self.manager = _manager()
|
||||||
|
|
||||||
|
def test_an_update_completing_after_the_call_is_still_reported(self):
|
||||||
|
"""The exact shape of the bug.
|
||||||
|
|
||||||
|
The enqueueing call sees nothing, because the worker has not run yet.
|
||||||
|
The next poll must report it -- under the old diff it was lost, since
|
||||||
|
the second snapshot was taken before the worker ever stamped.
|
||||||
|
"""
|
||||||
|
first = self.manager.drain_completed_updates()
|
||||||
|
self.assertEqual(first, [], "nothing has finished yet")
|
||||||
|
|
||||||
|
# The worker finishes some time later, on its own thread.
|
||||||
|
worker = threading.Thread(
|
||||||
|
target=self.manager._note_update_completed, args=("news",))
|
||||||
|
worker.start()
|
||||||
|
worker.join()
|
||||||
|
|
||||||
|
self.assertEqual(
|
||||||
|
self.manager.drain_completed_updates(), ["news"],
|
||||||
|
"an update that finishes between polls must still be reported")
|
||||||
|
|
||||||
|
def test_concurrent_completions_are_not_lost(self):
|
||||||
|
ids = ["plugin-%02d" % i for i in range(40)]
|
||||||
|
threads = [threading.Thread(target=self.manager._note_update_completed,
|
||||||
|
args=(pid,)) for pid in ids]
|
||||||
|
for thread in threads:
|
||||||
|
thread.start()
|
||||||
|
for thread in threads:
|
||||||
|
thread.join()
|
||||||
|
self.assertEqual(self.manager.drain_completed_updates(), sorted(ids))
|
||||||
|
|
||||||
|
def test_a_completion_during_a_drain_is_not_swallowed(self):
|
||||||
|
"""A drain must not clear an entry it did not report."""
|
||||||
|
self.manager._note_update_completed("news")
|
||||||
|
reported = self.manager.drain_completed_updates()
|
||||||
|
# ...worker finishes another one immediately afterwards
|
||||||
|
self.manager._note_update_completed("weather")
|
||||||
|
self.assertEqual(reported, ["news"])
|
||||||
|
self.assertEqual(self.manager.drain_completed_updates(), ["weather"])
|
||||||
|
|
||||||
|
|
||||||
|
class EveryStampRecordsACompletion(unittest.TestCase):
|
||||||
|
"""The ledger is only correct if the production paths actually fill it.
|
||||||
|
|
||||||
|
Asserting on the mechanics alone passes even when nothing calls
|
||||||
|
_note_update_completed -- verified by deleting the call sites, which the
|
||||||
|
behavioural tests above did not notice. This checks the invariant at the
|
||||||
|
source: wherever a successful update stamps plugin_last_update, it must
|
||||||
|
also record the completion, or Vegas silently stops being told.
|
||||||
|
"""
|
||||||
|
|
||||||
|
def test_success_paths_record_the_completion(self):
|
||||||
|
import src.plugin_system.plugin_manager as pm
|
||||||
|
|
||||||
|
tree = ast.parse(inspect.getsource(pm))
|
||||||
|
stamps = []
|
||||||
|
for node in ast.walk(tree):
|
||||||
|
if not isinstance(node, ast.With):
|
||||||
|
continue
|
||||||
|
# `with self._plugin_last_update_lock:` blocks that stamp a real
|
||||||
|
# time on success. Two stamps are deliberately excluded: the 0.0
|
||||||
|
# written at registration, and the failure path, which backs the
|
||||||
|
# timestamp off to space out retries -- neither means fresh data.
|
||||||
|
assigns_time = any(
|
||||||
|
isinstance(stmt, ast.Assign)
|
||||||
|
and any(isinstance(t, ast.Subscript)
|
||||||
|
and getattr(t.value, "attr", None) == "plugin_last_update"
|
||||||
|
for t in stmt.targets)
|
||||||
|
and not (isinstance(stmt.value, ast.Constant)
|
||||||
|
and stmt.value.value == 0.0)
|
||||||
|
and "failure" not in ast.dump(stmt.value)
|
||||||
|
for stmt in node.body
|
||||||
|
)
|
||||||
|
if assigns_time:
|
||||||
|
stamps.append(node)
|
||||||
|
|
||||||
|
self.assertGreaterEqual(
|
||||||
|
len(stamps), 2,
|
||||||
|
"expected the worker and inline success paths to stamp the time; "
|
||||||
|
"if this drops, the search below is looking at the wrong thing")
|
||||||
|
|
||||||
|
for stamp in stamps:
|
||||||
|
enclosing = self._enclosing_function(tree, stamp)
|
||||||
|
calls = [n for n in ast.walk(enclosing)
|
||||||
|
if isinstance(n, ast.Call)
|
||||||
|
and getattr(n.func, "attr", None) == "_note_update_completed"]
|
||||||
|
self.assertTrue(
|
||||||
|
calls,
|
||||||
|
"%s stamps plugin_last_update on success but never calls "
|
||||||
|
"_note_update_completed, so a plugin's fresh data would never "
|
||||||
|
"be reported and Vegas would keep its stale cached content"
|
||||||
|
% enclosing.name)
|
||||||
|
|
||||||
|
@staticmethod
|
||||||
|
def _enclosing_function(tree, target):
|
||||||
|
best = None
|
||||||
|
for node in ast.walk(tree):
|
||||||
|
if isinstance(node, (ast.FunctionDef, ast.AsyncFunctionDef)):
|
||||||
|
if node.lineno <= target.lineno <= (node.end_lineno or node.lineno):
|
||||||
|
if best is None or node.lineno > best.lineno:
|
||||||
|
best = node
|
||||||
|
return best
|
||||||
|
|
||||||
|
|
||||||
|
if __name__ == "__main__":
|
||||||
|
unittest.main(verbosity=2)
|
||||||
Reference in New Issue
Block a user