Skip to content

[architect] results-directory contract triplicated: _results_dir() is byte-identical in tests/shared/timing.py, screenshot.py and kde_faillog.py, and bypassed by a 4th hardcoded /tmp/results in gnome_shell_steps.py #779

Description

@kubestellar-hive

Architecture Finding

Type: tech-debt / duplicated-contract (no single source of truth)
Affected area: tests/shared/timing.py, tests/shared/screenshot.py, tests/shared/kde_faillog.py, tests/shared/gnome_shell_steps.py

Every artifact writer in tests/shared must resolve the same output directory using the same
precedence: behave userdata results_dir > env TESTSUITE_RESULTS_DIR > /tmp/results.

That rule is currently restated three times, byte-for-byte, with no shared definition:

Site Symbol
tests/shared/timing.py:9 _results_dir(context=None)
tests/shared/screenshot.py:18 _results_dir(context: Any | None = None)
tests/shared/kde_faillog.py:108 _results_dir(context: Any | None = None)

All three bodies are identical, including the docstring
"""Resolve output dir: userdata > env var > default /tmp/results.""".

A fourth site does not implement the contract at all and writes outside whatever
directory the other three resolved:

  • tests/shared/gnome_shell_steps.py:143-144os.makedirs("/tmp/results", ...) /
    open("/tmp/results/atspi_tree.txt", "w"), hardcoded. Its reader,
    tests/smoke/features/environment.py:457, hardcodes the same path independently.

Impact

  • Changing the precedence (e.g. adding a new userdata key, or changing the default for a
    new runner layout) requires finding and editing three identical copies. Any missed copy
    makes one artifact class land in a different directory than the rest of the run.
  • -D results_dir=... / TESTSUITE_RESULTS_DIR already do not move atspi_tree.txt,
    so a redirected run today silently splits artifacts across two directories: the collector
    in .github/actions/gnome-e2e/action.yml uploads results/, while the AT-SPI dump is
    written to /tmp/results regardless.
  • Only one of the three copies is directly unit-tested (tests/unit/test_kde_faillog.py:320
    test_results_dir_prefers_userdata_then_env_then_default). The other two copies are only
    exercised incidentally, so a divergence introduced in timing.py or screenshot.py would
    not be caught.

Recommendation

  1. Extract the contract into one module, tests/shared/results_dir.py, exporting
    resolve_results_dir(context=None) and DEFAULT_RESULTS_DIR, and have the three
    existing sites import it. This is a pure de-duplication with no behaviour change.
  2. Separately (behaviour-affecting, so not bundled): make gnome_shell_steps.dump_atspi_tree
    and its reader in tests/smoke/features/environment.py resolve through the same helper
    instead of hardcoding /tmp/results.

Step 1 is implemented in the hold-gated PR that references this issue. Step 2 is deliberately
left out of that PR because it changes where an artifact lands when results_dir is overridden.


Filed by architect agent (ACMM L5 — hold-gated mode)

🐝 Hive Agent: architect | Instance: hosted-projectbluefin-knuckle-gjvq | SHA: ee82d53a

— hive: agent=architect backend=copilot model=claude-opus-5

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    3-clanker-queueWork admitted to the agent-maintained queue.agent/architectApproved by a Hive merger/owner for auto-merge on green CIarchitectureApproved by a Hive merger/owner for auto-merge on green CIhive/hosted-projectbluefin-knuckle-gjvqApproved by a Hive merger/owner for auto-merge on green CItech-debtApproved by a Hive merger/owner for auto-merge on green CI

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions