Repository navigation
feat(repo): task host requirements, unsupported instead of a false pass (#41 D3) - #112
Conversation
…ss (intel#41 D3) task.json `host: {min_cpus, cpu_vendors, cpu_flags}` declares what a hidden test needs to show its bug. `repo tasks check` and `repo run` probe once, in a container of the evaluation's image, what a grading container sees (CPUs = affinity capped by the cgroup quota; vendor and flags from /proc/cpuinfo). An unmet task is `unsupported` with the reason in tasks check; in repo run its cells are not run but marked with unsupported.json, which repo report lists apart from failures and missing cells. The probed facts go to check.json, _eval/host.json and meta.json. A bad `host` shape is refused before any container starts. Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>
|
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #112 +/- ##
==========================================
+ Coverage 91.23% 91.30% +0.06%
==========================================
Files 213 216 +3
Lines 26357 26565 +208
Branches 4299 4337 +38
==========================================
+ Hits 24047 24254 +207
Misses 1643 1643
- Partials 667 668 +1
🚀 New features to boost your workflow:
|
With the flag, a task that is unsupported because the host misses its task.json `host` requirements is still printed with its reason but does not make the exit code 1; broken tasks and tasks that need egress still do. check.json records why a task is unsupported in `unsupported_by` (egress | host). The default is unchanged. Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>
There was a problem hiding this comment.
🟡 Changes recommended
The probe can miss container CPU quotas, and CLI requirement validation occurs after preflight containers start.
2 open issues
What changed in this PR
Adds task host requirements to repository evaluation, advancing #41’s D3 step so unsuitable hardware produces unsupported results rather than misleading grades.
Changes:
- Validates requirements and probes container-visible CPU capabilities.
- Skips unsupported tasks and records reasons and host facts.
- Adds CLI handling, reporting, documentation, and regression tests.
| File | Description |
|---|---|
| dokimi/tests/test_repo_host.py | Tests probing, validation, skipping, and reporting. |
| dokimi/dokimi/repos/selfcheck.py | Reports host-unsupported tasks. |
| dokimi/dokimi/repos/runner.py | Skips unsupported cells and records host facts. |
| dokimi/dokimi/repos/report.py | Separates unsupported cells from missing results. |
| dokimi/dokimi/repos/host.py | Implements requirements and capability probing. |
| dokimi/dokimi/commands/repo_run.py | Adds --allow-unsupported. |
| dokimi/docs/repo-evaluation-contract.md | Documents requirements and unsupported behavior. |
| dokimi/changelog.d/20261018_120000_napetrov_d3-host-requirements.md | Records the feature. |
Review effort: Balanced
| The host is probed once, before any cell (``_eval/host.json``); a cell of a task whose ``host`` | ||
| requirements it does not meet is marked unsupported instead of run. One that already has a | ||
| ``result.json`` (from a host that met them) keeps it, even with ``force``.""" | ||
| validate_tasks(tasks) |
There was a problem hiding this comment.
Fixed in 67d3a98: Host requirement validation now runs immediately after load_tasks() in _run_locked() before harness_version() and isolation_probe() calls. Malformed host requirements are rejected before any preflight containers start. Regression test strengthened to ensure no containers run before error, not just agent containers.
There was a problem hiding this comment.
Confirmed in 67d3a98: commands/repo_run.py calls validate_tasks right after load_tasks (line 182-183), before harness_version and isolation_probe, and the CLI test asserts no containers at all. No change in this round.
…arly in run Thread 1 (host.py line 57): The PROBE script now reads /proc/self/cgroup and /proc/self/mountinfo to correctly resolve CPU limits accounting for cgroup namespace visibility. Falls back to v1 paths for systems without cgroup v2. Regression test verifies the namespace-aware resolution logic is in place. Thread 2 (runner.py line 249): Host requirement validation now happens immediately after load_tasks() in _run_locked(), before harness_version() and isolation_probe() calls. This ensures malformed host requirements are rejected before any preflight containers start, not cell-by-cell during run_batch(). Regression test strengthens verification that no containers run before error. Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>
antonwolfy
left a comment
antonwolfy
left a comment
There was a problem hiding this comment.
The feature is sound and the flow is well covered, but the cgroup-namespace fix in the last commit does not work, and the test added for it cannot tell. Two risks and a doc nit are below.
What I ran, on a Linux review host at 67d3a98:
make ci-local: 19 passed, 0 failed, 4 skipped (docker/semgrep/deploy jobs).- Reproductions for the findings, with the container runner mocked.
- A mutation run over the new code: 32 mutants, 8 of which pass
test_repo_host.py,test_repo_runner.pyandtest_repo_run.py.
I agree with @copilot-pull-request-reviewer on host.py:58: the cgroup v2 branch is dead. New evidence: with /proc/self/cgroup = 0::/system.slice/x.scope, a cgroup2 mount and cpu.max = 200000 100000, the PROBE text returns cpu_max: None, so a private cgroup namespace works only through the old fixed-path fallback and the last commit changes nothing there.
Other findings
- The contract still shows the old
tasks checkusage. (outside this diff)docs/repo-evaluation-contract.md:47lists[--task <id>] [--warm]without--allow-unsupported, and line 670 saysexits 1 if any task is broken or unsupportedwith no exception. The contract also does not mention_eval/host.jsonorcheck.jsonhost, which the changelog does. Nit.
Checked, no issues
- Schemas: no schema or version covers
task.jsonhost,meta.jsonorcheck.jsonin this tree, so noSCHEMA_VERSIONSorschema_compatchange is needed;schema_compatpasses. - Marker handling:
_mark_unsupportedis skipped for a cell with aresult.json,load_unsupportedignores such cells, and--forcekeeps results on a small host. - Exit codes:
--allow-unsupportedexempts onlyunsupported_by == "host"; broken and egress tasks still exit 1 (mutants of this logic are killed). - Validation order:
validate_tasksin_run_lockedandrun_checkruns beforeharness_version,isolation_probeand the probe._context()can still run the image build first.
…res from host misses
- Fix cgroup resolver to split on at most 2 colons, correctly parsing v2 paths
like 0::/docker/eval (was: split("::"') breaks with extra colons).
- Change unmet() to return (reason, unsupported_by) tuple, distinguishing
'host-probe' (probe failed) from 'host' (hardware insufficient). This ensures
--allow-unsupported does not hide broken probes (e.g., python3 not in image).
- Add regression tests for cgroup v2 split and probe-failure marking.
Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>
…s test; untrack uv.lock Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>
There was a problem hiding this comment.
Warning
Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.
Copilot review overview
6 open issues
os.cpu_count()can returnNone(documented), which would serialize asnulland later produce… · Newtask.jsonwith"host": {}is treated as “has host requirements” (described as… · Newtask.jsonwith"host": {}is treated as “has host requirements” (described as… · Newunsupported_byisegressorhost, but the new behavior also returns… · New
Review effort: Lite
| cpuinfo = read("/proc/cpuinfo") or "" | ||
| cgroup_limits = resolve_cgroup_limits() | ||
| print(json.dumps({**cgroup_limits, "affinity": affinity, "cpuinfo": cpuinfo.strip().split("\n\n")[0]})) | ||
| """ |
There was a problem hiding this comment.
No change: a cpu count of None means the probe ran but could not tell, and unmet() deliberately reports that as host-probe (cannot tell), not as a hardware miss. Defaulting to 1 would let a min_cpus 1 task run on an unknown host and misreport a min_cpus 2 task as a hardware miss, so the fail-closed reason stays.
There was a problem hiding this comment.
🟡 Changes recommended
The verified cgroup-v1 probe failure overstates available CPUs, allowing unsupported tasks to run and potentially falsely pass.
6 open issues
task.jsonwith"host": {}is treated as “has host requirements” (described as…task.jsonwith"host": {}is treated as “has host requirements” (described as…os.cpu_count()can returnNone(documented), which would serialize asnulland later produce…unsupported_byisegressorhost, but the new behavior also returns…
1 resolved since last review
Review effort: Balanced
antonwolfy
left a comment
antonwolfy
left a comment
There was a problem hiding this comment.
Review at 7b1a22d. The probe fix is right: the v2 branch now resolves, the new tests run PROBE against fake trees, and a probe failure is now host-probe, which --allow-unsupported does not exempt for tasks check. One thing is still open on the repo run side, and one of my earlier test-gap points is only partly addressed.
What I ran, on a Linux review host at 7b1a22d:
make ci-local: 19 passed, 0 failed, 4 skipped (docker/semgrep/deploy jobs).- Reproductions for the code finding, with the container runner mocked.
- A mutation run over the new code: 49 mutants, 10 of which pass
test_repo_host.py,test_repo_runner.pyandtest_repo_run.py.
Findings
repo runexits 0 when the probe failed, so a broken image looks like a clean run. (commands/repo_run.py:207, outside this diff.) The exit check only looks atsummary["errors"]andsummary["budget_stopped"].run_batchmarks every task withhostrequirements unsupported on ahost-probefailure, as it does for a hardware miss, and_run_lockednever tells the two apart. With a fake runner whose probe exits 127 (python3: not found),repo run --task fix_raceprintsfix_race: UNSUPPORTED, its cells are not run: needs min_cpus 4, cpu_vendors GenuineIntel; cannot tell on this host: host probe exited 127 ... python3: not foundand then0 ran, 0 already done, 0 crashed, 0 not started (batch budget), exit 0. This is the case you fixed fortasks checkin 2fb4001. A CI job that runsrepo runon an image withoutpython3gets a green result with nothing graded. Suggestion: haverun_batchreturnunsupported_byper task and exit 1 from_run_lockedwhen any cell ishost-probe.test_run_skips_unsupported_cells_and_the_report_shows_themonly gets exit 0 becausefix_addalso runs; add a test with an all-unsupported batch and one with a failed probe.- Nit: the contract and the help text do not describe the final behaviour. (outside this diff)
docs/repo-evaluation-contract.md:47still liststasks check [--task <id>] [--warm]without--allow-unsupported, and line 670 saysexits 1 if any task is broken or unsupportedwith no exception. Thehost-probevalue ofunsupported_byand the fact that it is never exempted appear in no doc or changelog line._eval/host.json,unsupported.jsonandcheck.jsonhostare not in the contract's artifact lists either.
I agree with @copilot-pull-request-reviewer on host.py:129: "host": {} returns a default HostRequirements, so unmet() skips its early exit and a failed probe marks a task that needs nothing as host-probe, with the reason text needs nothing; cannot tell on this host. New evidence: {"min_cpus": 1} and {"cpu_vendors": []} behave the same.
Still open from my earlier review:
dokimi/tests/test_repo_host.py: the probe tests are fixed. The three mutants named in the thread still pass the suite:_mark_unsupportedwithout thermtree/mkdir(116 tests pass),load_unsupportedwithout theresult.jsonskip, andvalidate_tasksremoved fromrun_batch. The author deferred these, so I keep it open as a risk.
Checked, no issues
- Probe cgroup v2: run against fake trees through
DOKIMI_PROBE_ROOT: private namespace (0::/), host namespace, childmaxunder a limited ancestor, child limit above an ancestor limit, missingcpu.max. Each gives the expected CPU count. --allow-unsupported: exempts onlyunsupported_by == "host". Probe failure, egress and broken tasks still exit 1 fortasks check; mutants of this logic are killed.- Markers: a cell with a
result.jsonis kept with--forceon a small host.reportandcomparedo not count unsupported cells as missing. - Validation order:
validate_tasksnow runs right afterload_tasksin_run_locked, beforeharness_versionand the isolation probe. - Repo rules: no schema covers these files, changelog fragment present,
schema_compatandmkdocs buildpass, tests offline.
…e; empty host asks for nothing The probe walks the cpu controller of cgroup v1 the way it walks cgroup v2: the container's own cgroup and every visible ancestor, with the tightest quota winning, and a combined "cpu,cpuacct" mount is found from mountinfo. A task whose host object sets no constraint is treated as having no requirements, so a probe failure cannot mark it unsupported. The selfcheck docstring lists host-probe. Adds tests for the v1 layout, the empty host object, and the unsupported-cell bookkeeping and bad-host refusal in run_batch that were not covered. Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>
A failed probe left every task with host requirements unrun, yet the run exited 0, unlike tasks check. run_batch now returns the tasks the probe could not decide and _run_locked exits 1 on them; a real hardware miss still exits 0. Signed-off-by: Alexander Rybkin <alexander.rybkin@intel.com>
A marker went only when run_cell reached its cell, so a capable run cut short by the budget or an interrupt left the old host's marker, and repo report showed the cell as unsupported with that host's reason, not missing. run_batch now drops the markers of every planned cell this host supports. Signed-off-by: Alexander Rybkin <alexander.rybkin@intel.com>
No fake tree had two numeric limits, so flipping either comparison to loosest-wins passed the suite while a 2-CPU container read as 4. Signed-off-by: Alexander Rybkin <alexander.rybkin@intel.com>
AVX512F, as vendor docs write it, was a host miss on every machine and --allow-unsupported let it pass, so the task was never graded anywhere. cpuinfo flags are lower case, so folding cannot make a false match. Signed-off-by: Alexander Rybkin <alexander.rybkin@intel.com>
|
antonwolfy
left a comment
antonwolfy
left a comment
There was a problem hiding this comment.
Review at 3dd1a5c. The delta is right: the empty host object, the v1 combined-controller walk, the exit code on a failed probe, the stale-marker clearing and the case-insensitive flags each do what the commit says. Three things are worth fixing before merge, plus one nit.
What I ran, on a Linux review host at 3dd1a5c:
make ci-local: 19 passed, 0 failed, 4 skipped (docker/semgrep/deploy jobs). 5225 tests passed, 1 skipped.- Reproductions for the code findings, with the container runner mocked and the probe run against fake
/procand/systrees. - A mutation run over the new code: the tightest-quota comparisons (v1 and v2), the ancestor walks,
<to<=onmin_cpus, theprobe_failedexit condition and the marker-clearing loop are all caught. The ones that survive are in the inline comment on the tests.
Checked, no issues
- Marker clearing: it touches only
unsupported.jsonof the cells this invocation selects. Other tasks, arms and reps, and anyresult.json, are untouched. A capable run that the budget stops leaves the unreached cells missing, not unsupported. tasks check: a failed probe isunsupported_by=host-probe, exits 1 even with--allow-unsupported, and a task withouthoststaysok.- Quota maths: 150000/100000 gives 2, 100001/100000 gives 2, 50000/100000 gives 1. Mount lines with extra or missing optional fields, a
cpuset-only mount and a truncated line resolve correctly. - Repo rules: one changelog fragment, no generated artifacts, the contract page is already in
nav:, no schema covers these files.
…un is not a probe failure The probe joined the whole cgroup path onto the mount point, so a mount that shows only a subtree (mountinfo root /kubepods) read no limit. Strip the mount root first, for v1 and v2. repo run exited 1 on a complete re-run whenever the probe failed, because probe_failed ignored cells that already have a result. A task now counts only when one of its cells is left unrun. Also: the contract and changelog say unsupported cells carry host facts in unsupported.json, not meta.json; the probe tests no longer skip on small hosts and cover the cpuset mount, cpuset cgroup line, first cpuinfo block and upper-case host flags that the mutation check showed uncovered. Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>
antonwolfy
left a comment
antonwolfy
left a comment
There was a problem hiding this comment.
Review at 0862ecd. The mount-root walk and the probe_failed fix are right, and all four of my earlier threads are addressed. Two test gaps and one nit are left.
What I ran, on a Linux review host at 0862ecd:
make ci-local: 19 passed, 0 failed, 4 skipped (docker/semgrep/deploy jobs). Changed-line coverage 96%.- Probe runs against fake
/procand/systrees for root/, root equal to the cgroup path, root a prefix, root unrelated, root deeper than the path, trailing slashes and a sibling with a common string prefix, on v1 and v2. Every layout gives the right CPU count. - Mutation runs over the delta: removing
inside(), ignoring the root on either leg, and using mountinfo field 4 as the root are all caught.probe_failedwith the old condition and withany(_doneis caught. The two mutants named inline would pass the file, by reading the tests; I could not run them.
Checked, no issues
- Partial batch and
--force: a completefix_racebatch re-run with--forceand a failing probe exits 0 and keeps bothresult.jsonfiles. A batch with one finished and one unfinished cell exits 1, and the finished cell keeps its result. - Order:
_doneis evaluated before the stale-marker loop and before any cell runs, so the loop cannot changeprobe_failed. The budget stop and the unsupported path do not interact. - Docs: the contract's
hostrow and §4.3 sentence match the code, and the changelog fragment has the same wording.
…re-run Add probe cases where the mount root is a string prefix of the cgroup path (/kube vs /kubepods), v1 and v2, and a re-run where one of a task's cells is unfinished on a failed probe (exit 1). The probe-failure message now says the task has cells that were not run, since a finished cell is kept. Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>
antonwolfy
left a comment
antonwolfy
left a comment
There was a problem hiding this comment.
Review at cce629c. The three earlier threads are fixed: the /kube and /kubepods string-prefix tests, the partly finished batch test and the reworded message all do what the commit says. Two things are worth fixing before merge, both small.
What I ran, on a Linux review host at cce629c:
make ci-local: 19 passed, 0 failed, 4 skipped (docker/semgrep/deploy jobs). Changed-line coverage is above the 80% gate.- Mutation runs over the delta, against
test_repo_host.py(55 tests): dropping+ "/"ininside()fails the v1 and the v2 string-prefix test, and nothing else.any(not _done->all(not _donefails the new partial-batch test.any(not _done->any(_donefails two tests.return own->return ""ininside()fails both string-prefix tests. One mutant survives; it is in the inline comment. - Probe runs against fake
/procand/systrees for a trailing-slash mount root, a root equal to the path, an unparsablecpu.maxbelow a limited ancestor, and the ceil boundaries (150000/100000 gives 2, 100001/100000 gives 2, 50000/100000 gives 1). All give the right CPU count.
I agree with @xaleryb on cpu_vendors: ["Intel"] (it is exact and case-sensitive, and the contract does not say so) and on the contract usage lines 47 and 670, which still read as before at this head.
Checked, no issues
- Message: "have cells that were not run" appears only at
repo_run.py:208and in the test that asserts it. The contract and the changelog fragment do not quote it, so they stay accurate. - Lint:
ruff checkpasses on the changed files, including the longer print line. - Repo rules: no schema covers these files, one changelog fragment, SPDX headers present, no generated artifacts, tests offline.
| and any(not _done(batch_dir / cell_name(t.id, a, o.model, r)) | ||
| for t, a, o, r in cells if t.id == task_id)] |
There was a problem hiding this comment.
The if t.id == task_id filter in probe_failed is not pinned by any test. Mutation check: replacing for t, a, o, r in cells if t.id == task_id) with for t, a, o, r in cells) leaves all 55 tests in test_repo_host.py green.
Without the filter, one unfinished cell of any task flags every host-probe task. Case: fix_race (min_cpus 4) and fix_add (no host) both finished on a good host, then one fix_add result deleted and the batch re-run with a failing probe. fix_race has nothing left to run, and fix_add runs normally. With the mutant the run prints the host probe failed, so these tasks have cells that were not run: fix_race and exits 1. The unmutated code exits 0. The new partial-batch test only deletes a fix_race cell, so it cannot tell.
Suggestion: add a test beside test_a_partly_finished_batch_rerun_on_a_failed_probe_exits_1 that runs fix_add and fix_race on a good host, unlinks one fix_add result.json, re-runs with HostRunner(None), and asserts no host probe failed on stderr (no SystemExit). I ran exactly this: it passes on the head and fails on the mutant.
There was a problem hiding this comment.
Fixed in 11b5f54: added test_an_unfinished_cell_of_another_task_does_not_fail_a_task_that_is_done (fix_add and fix_race finished, one fix_add result deleted, re-run on a failing probe, exits 0). Dropping the t.id == task_id filter makes it fail; it passes now.
| check_p.add_argument("--allow-unsupported", action="store_true", dest="allow_unsupported", | ||
| help="Exit 0 when the only tasks that are not ok are unsupported because this host " | ||
| "misses their task.json `host` requirements (still printed with the reason). Broken " |
There was a problem hiding this comment.
The --allow-unsupported help says only broken and egress tasks still exit 1, but a failed host probe exits 1 too. repo_run.py:150 allows only unsupported_by == "host", so a task whose probe failed (host-probe, for example an image without python3) is not exempted.
A CI job that sets the flag on such an image fails with exit 1, and neither this help text, the contract (§4.3, "broken tasks and tasks that need egress still do") nor the changelog fragment says why. The distinction exists only in the unmet() and check_task docstrings.
Suggestion: add "and tasks whose host probe failed" to the help text, the contract sentence and the changelog line.
There was a problem hiding this comment.
Fixed in 11b5f54: the --allow-unsupported help, the contract sentence (§4.3) and the changelog line now say tasks whose host probe failed still exit 1.
…robe is not exempted by --allow-unsupported Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>
Re-added by mistake in 11b5f54; main does not track it. Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>
| reason, unsupported_by = _host_reason(ctx, task, work_dir, host) | ||
| if reason: | ||
| res.update(verdict="unsupported", unsupported_by=unsupported_by or "host", problems=[f"host: {reason}"]) | ||
| return res |
There was a problem hiding this comment.
On a host miss this returns before _prepare, the oracle and isolation_probe run, but only the null expectation depends on the hardware. Repro with the fake runner: RACE with gate ["tests_pas"] is BROKEN on 8 CPUs (exit 1), and on 2 CPUs with --allow-unsupported it is UNSUPPORTED and exits 0, though the help and the changelog say broken tasks still exit 1. So CI on a small runner never sees a broken grader of a host task. Could it still prepare the workspace and run isolation (and the oracle when only min_cpus is short), and skip only the null check? With --warm, the host check could be skipped, as the egress check is.
There was a problem hiding this comment.
Fixed in f2d3932. A host miss no longer returns early: the workspace is still prepared and isolation probed, and the oracle runs when only CPUs are short. Only the null check is skipped. --warm skips the host check, as it does egress. Tests: test_a_host_miss_still_checks_the_oracle_when_only_cpus_are_short, test_a_host_miss_still_probes_isolation_and_skips_only_null, test_warm_does_not_judge_the_host.
| host = probe_host(ctx, batch_dir / "_eval") | ||
| _write_json(batch_dir / "_eval" / "host.json", host) | ||
| verdicts = {t.id: unmet(t, host) for t in tasks} | ||
| unsupported = {task_id: why for task_id, (why, _) in verdicts.items() if why} |
There was a problem hiding this comment.
This dict also takes host-probe verdicts, so a failed probe writes unsupported.json into every cell of the task. repo run exits 1, but a later repo report on that batch says "0 missing" and lists the cells as "unsupported (not run, not failed)", and repo compare drops them from its missing count too. Nothing was measured there, so they should stay missing ("rerun the batch"). Could _mark_unsupported be skipped when unsupported_by is "host-probe"?
There was a problem hiding this comment.
Fixed in f2d3932. A host-probe failure writes no unsupported.json, so repo report/compare count the cells as missing. Test: test_a_probe_failure_leaves_no_marker_for_report_to_count.
| return ", ".join(parts) or "nothing" | ||
|
|
||
|
|
||
| def requirements(task: Task) -> HostRequirements | None: |
There was a problem hiding this comment.
repo validate does not call this, so task.json "host": {"min_cpu": 4, "cpu_flags": "avx2"} passes the static check (✓) and is refused only by tasks check / run. Could manifest._check_task call requirements() and add the HostSpecError text to the task's problems, so the static check and the run use the same rule?
There was a problem hiding this comment.
Fixed in f2d3932. manifest._check_task now calls requirements() and adds the HostSpecError text to the task's problems, so repo validate reports a bad host. Test: test_validate_reports_a_bad_host_shape.
| if not facts or facts.get("cpus") is None: | ||
| # A probe failure (error set, or no CPU count). Mark as "host-probe" so | ||
| # --allow-unsupported does not hide a broken measurement. | ||
| why = (facts or {}).get("error") or "the host probe reported no CPU count" | ||
| return (f"needs {need.describe()}; cannot tell on this host: {why}", "host-probe") |
There was a problem hiding this comment.
Only a missing CPU count is treated as "cannot tell". If /proc/cpuinfo could not be read, the facts have cpu_vendor None and cpu_flags [], and a task with cpu_flags or cpu_vendors becomes a "host" miss that --allow-unsupported passes. Could it return "host-probe" when the task needs vendor or flags and the probe gave neither?
There was a problem hiding this comment.
Fixed in f2d3932. A task that needs a vendor or flags, when the probe read neither from /proc/cpuinfo, now returns host-probe instead of a hardware miss. Test: test_missing_cpuinfo_is_a_probe_failure_not_a_miss.
| short = [] | ||
| if facts["cpus"] < need.min_cpus: | ||
| short.append(f"{facts['cpus']} CPU(s) < min_cpus {need.min_cpus}") | ||
| if need.cpu_vendors and facts.get("cpu_vendor") not in need.cpu_vendors: |
There was a problem hiding this comment.
Flags are matched in any case, vendors exactly, so cpu_vendors ["Intel"] or ["genuineintel"] is a
host miss on every machine, and --allow-unsupported hides it. Could vendors be compared case-insensitively too, and an unknown vendor name (not GenuineIntel, AuthenticAMD, ...) be refused by requirements()?
There was a problem hiding this comment.
Fixed in f2d3932. Vendors compare case-insensitively, and requirements() refuses names outside GenuineIntel, AuthenticAMD, HygonGenuine, CentaurHauls and Shanghai. Contract doc and changelog updated. Test: test_vendors_compare_in_any_case_and_unknown_names_are_refused.
…lation, probe-failure markers, validate, cpuinfo, vendor case Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>
…ifest does not import host manifest validated task.json host by importing host (and workspace) inside a function, which grew the bases/manifest import cycle. host_spec parses the raw value, errors holds PrepError for both workspace and host_spec; host re-exports. Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>
…requirements Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com> # Conflicts: # dokimi/dokimi/repos/report.py
…record, both new arms in the README (#199) Leftovers from the reviews of #117 and #112: - The contract said every character outside [A-Za-z0-9.-] becomes `_`. The code turns each run into one `_`, drops a leading or trailing `_` and cuts at 40, so `my fix (v2).diff` is `my_fix_v2_.diff`. Also re-wrap the paragraph's 130-character line. - The S5 usage of `repo tasks check` lacked `--allow-unsupported`, and the example's "exits 1 if any task is unsupported" had no exception for it. - §4.3 names where the host probe's facts go: `check.json` `host` and the batch's `_eval/host.json`. - The README's arm table lists `guidance:rev:<rev>` and `patch:<file>`. Signed-off-by: Anton Volkov <antonwolfy@gmail.com>
Part of #41 (step D3).
Summary
A task can now say what hardware its hidden test needs to show the bug. On a host that misses it, the task is reported as unsupported instead of getting a false pass or a false "grader broken". The motivating case is a oneDAL thread race that only reproduces when oneTBB sees 3 or more CPUs, and was never reproduced on AMD EPYC.
All three keys are optional. The check uses what a container of the evaluation sees, not what the host has:
dokimi/repos/ +├── host.py # requirements(), probe_host(), host_facts(), unmet() ├── selfcheck.py # unsupported verdict; check.json gains "host" ├── runner.py # unsupported cells skipped and marked; _eval/host.json; meta.json "host" └── report.py # unsupported cells are not missing; listed with their reasontasks checkexits 1 for an unsupported task, the same as it already does for a task that needs egress. With--allow-unsupported, host-unsupported tasks are still printed with their reason but don't make the exit code 1; broken tasks and egress tasks still do. This is for CI on hosts that cannot show some bugs, like oneDAL's old SKIPPED.check.jsonrecordsunsupported_by: egress | host.repo runexits 0 when the only unrun cells are unsupported ones.result.jsonkeeps it on a host that misses the requirements, even with--force.python3), a task withhostrequirements is unsupported with the reason "cannot tell on this host". A task without requirements is not affected.Evidence
New
tests/test_repo_host.py(29 tests, no docker). The key ones:Before: on
mainwith onlyhost.pyadded, the 4 check/run/report tests fail: the race task is checked and run as usual, and there is nounsupported.jsonand nohostincheck.json. The rest fail to import withouthost.py.Without the flag's code, the 3
--allow-unsupportedtests fail (unrecognized arguments: --allow-unsupported).After: all 29 pass. Full suite: 5207 passed, 2 skipped.
ruff,mypy,schema_check,bandit -ll, pre-commit and the changelog fragment check all pass. Every new or touched function is at radon CC ≤ 15 exceptrun_cell(18, unchanged frommain) andsummarize(25, unchanged).Real docker smoke of
probe_hostinpython:3.12-slimon a 224-thread Xeon host:docker runflag--cpuset-cpus 0-1--cpus 2--cpus 1.5Merge Danger
Door: two-way.
hostis a new optional key, so a task without it behaves as before. The new files (unsupported.json,_eval/host.json) and the new keys (meta.jsonhost,check.jsonhost) are additions.Blast Radius: small
tasks checkandrepo runnow starts one extra short container, the probe, even when no task declareshost. This is how the facts get recorded. If the image has nopython3, the probe failure is recorded and only tasks withhostrequirements are affected.unsupportedandunsupported_tasks. Therepo runcommand line still prints the old counts, and unsupported tasks are logged one line each.repo reportandrepo comparestop counting unsupported cells as missing.repo validate(manifest.py) does not checkhostyet;tasks checkandrundo.Checklist
pytestpassesruff,mypy, and schema checks passtask.jsontable and §4.3 Self-check.github/.licenserc.yaml)🤖 Generated with Claude Code