Skip to content

Restore existing result loading in allure watch - #976

Open
spetrianni wants to merge 5 commits into
allure-framework:mainfrom
spetrianni:fix/watch-initial-results
Open

spetrianni wants to merge 5 commits into
allure-framework:mainfrom
spetrianni:fix/watch-initial-results

Conversation

@spetrianni

@spetrianni spetrianni commented Sep 15, 2026 •

Copy link
Copy Markdown

Context

allure watch can start with an empty report even when valid results already exist in the selected directories.

The regression was introduced in #816 (76343c89): --new-only was added with a default of true. Consequently, the initial file scan used ignoreInitial: true, indexed the existing files without calling AllureReport.readResult, and never picked those files up on subsequent scans. This affected name-based discovery, explicit directories, CLI globs, and config.resultsDir patterns.

Changes

  • Restore the previous startup behavior by making --new-only opt-in (false by default).
  • Preserve explicit --new-only and --no-new-only behavior, live ingestion, and the existing rule that directories discovered after startup always load their backlog.
  • Update CLI help and the CLI README to describe these semantics.
  • Expand regression coverage across discovery modes and flags, including multiple startup directories, directories created later, and an initially empty discovery scan.
  • Exercise the real filesystem/file watcher with existing and newly written result files. Assert report ingestion and absence of duplicate ingestion; clean up temporary directories, watchers, and test-created exit listeners.
Invocation Results present at startup Results written later
allure watch Loaded Loaded
allure watch --new-only Skipped Loaded
allure watch --no-new-only Loaded Loaded

No changes to the reader, report rendering, shared watcher implementation, or run/agent defaults are needed.

Regression evidence

Following AGENTS.md and docs/allure-test-agent.md, test executions used Allure agent mode with fresh managed output directories and explicit scope expectations.

Before the fix, the new watch coverage produced 5 failures and 18 passes: all four default-mode discovery variants incorrectly received ignoreInitial: true, and the real file-watcher scenario did not ingest the existing result. Explicit flag scenarios passed. The failed run also recorded the expected nonzero test-process global error; its stderr and per-test evidence were reviewed, rather than treating the partial modeling status as a success.

After the fix, the focused run produced 24 passed, 0 failed, 0 skipped, exit 0. Expected scope matched; the final runtime model was complete and the findings manifest was empty. Reviewed index.md, manifest/run.json, manifest/test-events.jsonl, manifest/tests.jsonl, manifest/findings.jsonl, and the relevant per-test steps.

The real watcher now forwards one existing result and one live result by default; with --new-only, it forwards only the live result. --no-new-only forwards both. All three scenarios assert no duplicate ingestion during shutdown.

yarn allure agent --report off --goal "Watch loads existing results by default while preserving explicit new-only flags, live ingestion and directory discovery" --expect-tests 24 --expect-label module=cli --results-dir .\packages\cli\out\allure-results -- yarn workspace allure test watch.test.ts resultsDiscovery.test.ts
yarn workspace allure build

Locally, Yarn was invoked through the repository-pinned node .\.yarn\releases\yarn-4.15.0.cjs launcher because a global Yarn executable was unavailable. The initial root workspace build completed successfully, and the changed CLI package was rebuilt after the fix. Targeted standard Oxlint and Oxfmt checks, plus the normal pre-commit hooks, completed successfully.

Validation limits

This is focused CLI/file-watcher coverage, not a full-suite, cross-platform, or browser-rendering claim. The ingestion tests use the real file watcher but mock report generation and the HTTP server. Local execution was on Windows with Node.js 26.8.2 and Allure 3.17.0.

The additional native oxlint --type-aware attempt could not complete: it reported unresolved Node type definitions and a rootDir/common-source-directory configuration diagnostic in this local Yarn PnP checkout. It is not counted as passing validation; the CLI's existing TypeScript build did pass. No unrelated configuration changes were made to work around it.

Checklist

Make --new-only opt-in again, document startup behavior, and cover initial and live ingestion across discovery modes.

Co-authored-by: Copilot <[email protected]>
@CLAassistant

CLAassistant commented Sep 15, 2026 •

Copy link
Copy Markdown

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you all sign our Contributor License Agreement before we can accept your contribution.
1 out of 2 committers have signed the CLA.

✅ spetrianni
❌ Copilot
You have signed the CLA already but the status is still pending? Let us recheck it.

@todti

todti commented Sep 16, 2026

Copy link
Copy Markdown
Collaborator

Thanks for the detailed writeup and the regression tests. The diagnosis is exactly right: ignoreInitial: true indexes the pre-existing files without ever reading them, so they stay invisible for the rest of the session. That part is a genuine bug.

The default flip is the piece I'd like to reconsider, though.

--new-only defaulting to true wasn't incidental. allure watch with no arguments falls back to name-based discovery: it walks the cwd up to depth 10 and picks up every allure-results directory in the repository. On a large monorepo, or on a checkout with accumulated results, that backlog can be enormous, and parsing all of it before the report shows anything is the exact cost the flag was introduced to avoid. Flipping the default puts that cost back on the most common invocation.

That said, the current behaviour fails badly: the report is silently empty, and there is no way for the user to tell that a backlog was skipped rather than that something is broken. Documenting --no-new-only doesn't fix that, since nobody looks up a recipe for a problem they don't know they have.

So I'd suggest keeping the default and making the skip visible instead. The phase is already known in watch.ts without ignoreInitial:

let skippingBacklog = this.newOnly && isInitialDiscovery;

const watcher = newFilesInDirectoryWatcher(newDir, async (path) => {
  if (skippingBacklog) {
    skippedResults++;
    return;
  }
  await allureReport.readResult(new PathResultFile(path));
});

await watcher.initialScan();
skippingBacklog = false;

and after the discovery watcher's initial scan:

if (skippedResults > 0) {
  console.info(`skipped ${skippedResults} existing result(s); pass --no-new-only to load them`);
}

The progress plugin only clears its own line and explicitly protects foreign writes, so this stays in the scrollback and remains visible for the whole session.

Concretely, what I'd like to keep from this PR:

  • the README paragraph, which describes the semantics well, just with --new-only still on by default;
  • the regression tests, which stay valuable either way (the explicit-flag scenarios are unchanged, and the default-mode ones flip their expectation to "skipped, and reported").

And drop the one-line default change in favour of the counter plus the startup notice.

If you'd rather not extend the scope, I'm happy to take the counter part in a follow-up and land the docs and tests from here first.

Copilot AI and others added 4 commits September 17, 2026 07:50

@spetrianni spetrianni left a comment •

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@todti Agreed with your suggestions, please proceed with the review. I left the default behavior set to new-only

This branch has not been deployed

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants