Skip to content

mrc-6988 fix output buttons bug - #45

Open
EmmaLRussell wants to merge 2 commits into
mainfrom
mrc-6988-fix-output-buttons
Open

EmmaLRussell wants to merge 2 commits into
mainfrom
mrc-6988-fix-output-buttons

Conversation

@EmmaLRussell

Copy link
Copy Markdown
Collaborator

I noticed that the open output buttons had stopped working and the error logged in the server suggested that it had been trying to open a folder name corresponding to a big chunk of the output log.

I'm not quite sure why the logging from Piranha has changed, but it seems that the regex I use in piranhaAPI to determine the name of the output log had been way too greedy and the match had gobbled multiple lines rather than doing a lazy match. I've joined the log with newlines rather than spaces so that the match should never traverse lines and have also explicitly made the match lazy.

This is awkward to put in an e2e test for as it's about opening other programs - best to just test manually that the "Open report" and "Open output folder" buttons work at the end of a run.

@EmmaLRussell
EmmaLRussell marked this pull request as ready for review September 8, 2026 15:04

@david-mears-2 david-mears-2 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Seems to work, though the logs my job generated wouldn't have failed with the greedy regex in any case (they only listed the summary report in one place).

For unit testing, would it work to update expectedLog in piranha-apps/svelte-app/tests/unit/lib/piranhaAPI.svelte.test.ts to have an adversarial example, i.e. one where greedy-regex would go wrong and lazy-regex wouldn't? And then assert that window.api.openRunOutputFolder was called with the correct parameters?

Current:

const expectedLog = [
      "test log message 1",
      "test log message 2",
      "Generated: /data/run_data/output/output_1/report.html",
    ];

Proposed:

const expectedLog = [
      "test log message 1",
      "test log message 2",
      "Generated: /data/run_data/output/output_1/report.html",
      "Honeypot: /data/run_data/output/honeypot_for_greedy_regex/report.html",
    ];

/\/data\/run_data\/output\/(.*?)\/report\.html/,
);
if (match) {
this.#runOutputFolderName = match[1];

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Why index of 1?

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants