A7's "do not" is louder than its "do", so that first: admin.js:484-517 handles
language and time, it is a shipped feature, the review specifically endorsed
keeping it, and if the proposal reads as removing it that reading is wrong. It
is UNTOUCHED. The probe checks that by diffing the file against HEAD as well as
by driving the feature — switching locale, saving, reloading, and confirming the
saved value came back from the server.
Card status lines
Two things were doing one job badly. The SOP card said "SOP complete" when it
was and NOTHING when it was not — so the commonest state on a live project was
the one with no status line at all. And the Work Package card carried its state
in its BUTTON ("Complete SOP first", "Checking..."), which is a button
describing a situation instead of naming what pressing it does.
Now every card says its state in its own line, in all three states, and no
button changes text to report one:
complete green, the canonical success token
not yet secondary text - a real answer, and neither green nor a warning
unknown the suite's amber, and it names the failure
Each carries a glyph and a word as well as a colour. The line is replaced in
place rather than removed and re-added, because a card that briefly has no
status line reads as "no status" and that is one of the three real answers.
role="status" on it: the text is written by a fetch that lands after the page
has settled, which is what aria-live is for (S10).
Footer
Was "Work Package Suite v1.0 | Prime Controls - Business Technology Group |
Pilot Use Only", which leaves three questions open: v1.0 of what, who Business
Technology Group is to this page, and what Pilot Use Only actually restricts.
Now two sentences. The first names the product and who maintains it. The
second says what "pilot" means in the only terms that matter to somebody about
to type a real work package into it: the work is real and is kept, the tools
around it are still changing. The bare version string is gone rather than left
claiming to be a version of something unspecified.
html/index.html three-state card status, the footer
tests/cards_check.py new - 44 checks
tests/aggregates_check.py two assertions re-pointed (see below)
Done when
[x] card status lines read clearly and use the canonical status colours
[x] the footer is unambiguous about what it is showing
[x] localization still functions - verified by switching language, saving,
reloading and reading the value back off the server
[x] admin.js:484-517 behaviour is unchanged - and the file is byte-identical
Two probes needed re-pointing, and both were asserting wording rather than
behaviour
aggregates_check waited for the card's status line to be non-empty and then
matched the phrase "SOP complete". The wait is now wrong for a second reason:
the line is non-empty from the moment the page loads, because it says
"Checking the SOP...". It waits for the answer instead, and matches the
ANSWER rather than the sentence - which is what that check was ever about,
since it exists to prove the answer came from the server and not the cache.
A probe that breaks when wording changes is a probe that will be edited
carelessly the next time wording changes. Both are now written so that only a
behaviour change can fail them.
Verified one at a time
cards_check 44/44 new
aggregates 16/16 (two assertions re-pointed)
browser_check 71/71
a11y 22/22
launcher 58/58
f_items F1-F5 FIXED, F6 REPRODUCES (T7.2)
No colour literal added: the three status colours are --cds-support-success,
--cds-text-secondary and --wp-status-warning-text, all already in
theme-light.css.
Question for the PR, per CLAUDE.md: the footer now says work packages created in
the pilot are kept. That is true of the database and it is the thing people
actually want to know, but it is a promise, and whoever owns the pilot should
confirm it is one we are making.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
270 lines
13 KiB
Python
270 lines
13 KiB
Python
#!/usr/bin/env python3
|
|
"""Do the counts come from the server? — B4 / T4.1.
|
|
|
|
The defect B4 names is not "the numbers are wrong". It is that they were derived
|
|
from the caller's own localStorage, so two people on the same project saw
|
|
different numbers and neither was told. A test that only checks the totals are
|
|
correct would have passed before this change, because on one browser with one
|
|
cache they were correct.
|
|
|
|
So this checks the thing that was actually broken:
|
|
|
|
1. the same project reports the same aggregates to two different users
|
|
2. the dashboard shows the SERVER's total even when the browser's own cache has
|
|
been poisoned with a different one — which it cannot do if it is summing
|
|
localStorage
|
|
3. a failed aggregate request renders an explicit error and a retry, not a zero
|
|
and not the last good answer
|
|
4. the launcher's SOP status survives a poisoned cache the same way
|
|
5. the aggregate response can be grouped by location without a schema change
|
|
|
|
Self-contained: throwaway SQLite, its own uvicorn, headless Edge or Chrome over
|
|
CDP, all torn down. Exit 0 all passed, 1 a failure, 2 could not run.
|
|
"""
|
|
import json
|
|
import os
|
|
import subprocess
|
|
import sys
|
|
import tempfile
|
|
import time
|
|
import urllib.error
|
|
import urllib.request
|
|
|
|
sys.path.insert(0, os.path.dirname(os.path.dirname(os.path.abspath(__file__))))
|
|
sys.path.insert(0, os.path.dirname(os.path.abspath(__file__)))
|
|
|
|
import cdp # noqa: E402
|
|
from browser_check import seed, start_server, chk, _PASS, _FAIL, _c # noqa: E402
|
|
|
|
|
|
def api(base, path, token, method="GET", body=None):
|
|
req = urllib.request.Request(base + path, method=method)
|
|
req.add_header("Cookie", "wp_session=" + token)
|
|
req.add_header("Accept", "application/json")
|
|
data = None
|
|
if body is not None:
|
|
data = json.dumps(body).encode()
|
|
req.add_header("Content-Type", "application/json")
|
|
with urllib.request.urlopen(req, data, timeout=15) as r:
|
|
return json.loads(r.read().decode() or "null")
|
|
|
|
|
|
def main():
|
|
exe = cdp.find_browser()
|
|
if not exe:
|
|
print("no headless-capable browser found; set WP_BROWSER.")
|
|
return 2
|
|
|
|
tmpdir = tempfile.mkdtemp(prefix="wpsuite-aggregates-")
|
|
db_path = os.path.join(tmpdir, "check.db")
|
|
server = None
|
|
try:
|
|
tok = seed(db_path)
|
|
port = cdp.free_port()
|
|
base = "http://127.0.0.1:%d" % port
|
|
server = start_server(port, db_path)
|
|
if server is None:
|
|
print("the test server would not start.")
|
|
return 2
|
|
print("\nAggregate counts — B4 / T4.1\nTarget: %s" % base)
|
|
|
|
# Extra packages so the counts are not all the same number: one on hold,
|
|
# one blocked by an open constraint, one closed, each with a location.
|
|
for i, (num, status, constraint, loc, hours) in enumerate([
|
|
("WP03-HOLD", "Issue", "cleared", "B100 / L2 / P", "12"),
|
|
("WP04-GATE", "Draft", "open", "B100 / L2 / P", "8"),
|
|
("WP05-DONE", "Closed", "cleared", "B100 / L3 / Q", "20"),
|
|
]):
|
|
api(base, "/api/wps", tok["root"], "POST", {
|
|
"id": "wpX%d" % i, "project_id": "projA", "sop_id": "sopA",
|
|
"number": num, "subject": num, "type": "Conduit Install",
|
|
"status": status,
|
|
"data": {"disciplines": ["Electrical"], "hours": hours,
|
|
"location": loc,
|
|
"constraints": [{"name": "Materials", "status": constraint,
|
|
"comment": "waiting on delivery"}]},
|
|
})
|
|
|
|
print("\n1. the same project reports the same aggregates to two users")
|
|
as_root = api(base, "/api/wps/metrics?project_id=projA", tok["root"])
|
|
as_pat = api(base, "/api/wps/metrics?project_id=projA", tok["pat"])
|
|
comparable = ["total", "release_ready", "on_hold", "overdue",
|
|
"est_hours", "actual_hours", "by_status", "by_discipline"]
|
|
same = {k: as_root[k] for k in comparable} == {k: as_pat[k] for k in comparable}
|
|
chk("root and pat get identical aggregates for projA", same,
|
|
"root=%s pat=%s" % ({k: as_root[k] for k in comparable},
|
|
{k: as_pat[k] for k in comparable}))
|
|
chk("the fixture is not degenerate (>=5 packages, a hold, a gate)",
|
|
as_root["total"] >= 5 and as_root["on_hold"] >= 1 and len(as_root["gating"]) >= 1,
|
|
"total=%s on_hold=%s gating=%s" % (as_root["total"], as_root["on_hold"],
|
|
len(as_root["gating"])))
|
|
chk("'mine' is per-user, so it is allowed to differ",
|
|
"mine" in as_root and "mine" in as_pat)
|
|
|
|
print("\n5. the aggregate can be grouped by location without a schema change")
|
|
loc = as_root.get("by_location") or {}
|
|
chk("by_location carries its dimensions", isinstance(loc.get("dimensions"), list)
|
|
and len(loc["dimensions"]) >= 1, loc.get("dimensions"))
|
|
groups = loc.get("groups") or []
|
|
chk("by_location groups are keyed by those dimensions",
|
|
bool(groups) and all(set(g["key"]) == set(loc["dimensions"]) for g in groups),
|
|
[g.get("key") for g in groups[:3]])
|
|
chk("each group carries its own rollup, not just a count",
|
|
bool(groups) and all({"total", "release_ready", "on_hold", "by_status"} <= set(g)
|
|
for g in groups),
|
|
list(groups[0]) if groups else None)
|
|
chk("the groups sum to the project total",
|
|
sum(g["total"] for g in groups) == as_root["total"],
|
|
"%s vs %s" % (sum(g["total"] for g in groups), as_root["total"]))
|
|
|
|
browser = cdp.Browser(exe)
|
|
page = browser.page()
|
|
try:
|
|
print("\n2. the dashboard shows the server's total, not the browser's")
|
|
page.clear_cookies()
|
|
page.set_cookie("wp_session", tok["root"])
|
|
page.goto(base + "/wp-creation-index.html?project=projA")
|
|
time.sleep(1.5)
|
|
# Poison this browser's cache with a different number of packages than
|
|
# the server has. If any displayed count still tracks localStorage, it
|
|
# will report 2 and the server's total will not match.
|
|
page.eval("""(() => {
|
|
const fake = [
|
|
{id:'fake1', number:'FAKE-1', subject:'not on the server', status:'Draft',
|
|
disciplines:['Electrical'], hours:'999', constraints:[]},
|
|
{id:'fake2', number:'FAKE-2', subject:'also not', status:'Draft',
|
|
disciplines:['Electrical'], hours:'999', constraints:[]}
|
|
];
|
|
localStorage.setItem('wp_iwp_v1::projA', JSON.stringify(fake));
|
|
localStorage.setItem('wp_iwp_v1', JSON.stringify(fake));
|
|
return true;
|
|
})()""")
|
|
page.goto(base + "/wp-creation-index.html?project=projA&view=dashboard")
|
|
time.sleep(1.2)
|
|
page.eval("typeof showDashboard==='function' && showDashboard()")
|
|
for _ in range(30):
|
|
ready = page.eval("!!document.querySelector('.dash-metric .dm-val')")
|
|
if ready:
|
|
break
|
|
time.sleep(0.3)
|
|
shown = page.eval(
|
|
"(()=>{const e=[...document.querySelectorAll('.dash-metric')]"
|
|
".find(x=>/Total WPs/i.test(x.textContent));"
|
|
"return e?e.querySelector('.dm-val').textContent.trim():null})()")
|
|
chk("dashboard 'Total WPs' equals the server total",
|
|
str(shown) == str(as_root["total"]),
|
|
"shown=%r server=%r (poisoned cache said 2)" % (shown, as_root["total"]))
|
|
chk("...and is therefore not the poisoned cache's 2", str(shown) != "2", shown)
|
|
|
|
print("\n3. a failed aggregate request is an error, not a zero")
|
|
page.eval("""(() => {
|
|
const real = window.fetch;
|
|
window.fetch = function(u, o){
|
|
if (String(u).indexOf('/api/wps/metrics') !== -1)
|
|
return Promise.reject(new Error('simulated outage'));
|
|
return real.apply(this, arguments);
|
|
};
|
|
dashMetrics = null; dashMetricsErr = null;
|
|
loadDashMetrics();
|
|
return true;
|
|
})()""")
|
|
time.sleep(1.2)
|
|
txt = page.eval("(document.getElementById('dashboard-view')||{}).textContent||''")
|
|
chk("an explicit error panel is shown", "Counts unavailable" in txt, txt[:120])
|
|
chk("...naming the failure", "simulated outage" in txt, txt[:160])
|
|
chk("...offering a retry",
|
|
page.eval("!!document.querySelector('#dashboard-view button')"))
|
|
chk("no metric tiles are rendered alongside the error",
|
|
page.eval("document.querySelectorAll('#dashboard-view .dash-metric').length") == 0)
|
|
chk("the error region announces itself",
|
|
page.eval("!!document.querySelector('#dashboard-view [role=alert]')"))
|
|
|
|
print("\n4. the launcher's SOP status comes from the server too")
|
|
page.goto(base + "/index.html")
|
|
time.sleep(0.6)
|
|
page.eval("""(() => {
|
|
try {
|
|
localStorage.setItem('wp_active_project_id', 'projA');
|
|
localStorage.setItem('wp_suite_sop_complete::projA', '0');
|
|
localStorage.removeItem('wp_suite_sop::projA');
|
|
} catch(e){}
|
|
return true;
|
|
})()""")
|
|
page.goto(base + "/index.html")
|
|
# A7/T6.5 gave the card a status line in EVERY state, including while
|
|
# the request is in flight ("Checking the SOP..."). So waiting for the
|
|
# line to be non-empty is no longer waiting for the answer — wait for
|
|
# it to stop saying it is checking.
|
|
for _ in range(30):
|
|
s = page.eval("(document.querySelector('#card-sop .card-status')||{}).textContent||''")
|
|
if s and "Checking" not in s:
|
|
break
|
|
time.sleep(0.3)
|
|
status = page.eval(
|
|
"(document.querySelector('#card-sop .card-status')||{}).textContent||''")
|
|
# The wording moved at T6.5 ("SOP complete" -> "Complete - <name>").
|
|
# What this check is about is WHERE the answer came from, not how it is
|
|
# phrased, so it asserts the answer and not the sentence.
|
|
chk("launcher reports the SOP complete despite a cache that says otherwise",
|
|
"Complete" in status and "Not finished" not in status,
|
|
"card said %r" % status.encode("ascii", "replace").decode("ascii"))
|
|
|
|
page.goto(base + "/index.html")
|
|
time.sleep(0.4)
|
|
page.eval("""(() => {
|
|
const real = window.fetch;
|
|
window.fetch = function(u, o){
|
|
if (String(u).indexOf('/summary') !== -1)
|
|
return Promise.reject(new Error('simulated outage'));
|
|
return real.apply(this, arguments);
|
|
};
|
|
return true;
|
|
})()""")
|
|
page.eval("typeof applyActiveProject==='function' && applyActiveProject()")
|
|
time.sleep(1.0)
|
|
status = page.eval(
|
|
"(document.querySelector('#card-sop .card-status')||{}).textContent||''")
|
|
# Same again: the wording moved at T6.5 ("Could not check SOP status"
|
|
# -> "Unknown - could not reach the server"). The assertion is that the
|
|
# card states the failure rather than guessing, in whatever words.
|
|
chk("a failed status request says so on the card",
|
|
"Unknown" in status and "could not reach" in status.lower(),
|
|
"card said %r" % status.encode("ascii", "replace").decode("ascii"))
|
|
finally:
|
|
page.close()
|
|
browser.close()
|
|
except urllib.error.HTTPError as e:
|
|
print("API error: %s %s" % (e.code, e.read()[:300]))
|
|
return 2
|
|
finally:
|
|
if server:
|
|
server.kill()
|
|
try:
|
|
server.wait(timeout=10)
|
|
except subprocess.TimeoutExpired:
|
|
pass
|
|
try:
|
|
from server.db import engine
|
|
engine.dispose()
|
|
except Exception:
|
|
pass
|
|
import shutil
|
|
for _ in range(10):
|
|
shutil.rmtree(tmpdir, ignore_errors=True)
|
|
if not os.path.exists(tmpdir):
|
|
break
|
|
time.sleep(0.3)
|
|
|
|
total = len(_PASS) + len(_FAIL)
|
|
print("\n%s\n%d/%d checks passed." % ("-" * 54, len(_PASS), total))
|
|
if _FAIL:
|
|
for f in _FAIL:
|
|
print(" - " + f)
|
|
return 1
|
|
print("\nResult: " + _c("ALL PASS — counts come from the server.", "32") + "\n")
|
|
return 0
|
|
|
|
|
|
if __name__ == "__main__":
|
|
sys.exit(main())
|