Skip to content

runtests: Added runnernums in order to have O(1) lookup - #22753

Closed
mihaigmarin wants to merge 4 commits into
curl:masterfrom
mihaigmarin:feature/runtests
Closed

mihaigmarin wants to merge 4 commits into
curl:masterfrom
mihaigmarin:feature/runtests

Conversation

@mihaigmarin

Copy link
Copy Markdown
Contributor

This pull request adds runnernums the reverse of runnerids.
In this way the iteration over runnerids is no longer needed.
The check can happen directly in runnernums.

This pull request adds runnernums the reverse of runnerids.
In this way the iteration over runnerids is no longer needed.
The check can happen directly in runnernums.

Signed-off-by: Mihai-Gabriel Marin <mihaigmarin@gmail.com>

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟢 Approval recommended

Pull request overview

Adds a reverse runner-ID lookup to avoid repeated linear searches when resolving runner log directories.

Changes:

  • Introduces %runnernums.
  • Populates both runner lookup maps during initialization.
  • Uses constant-time lookup in getrunnerlogdir.
File summaries
File Description
tests/runtests.pl Adds and uses the reverse runner lookup map.
Review details
  • Files reviewed: 1/1 changed files
  • Comments generated: 0
  • Review effort level: Balanced

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread tests/runtests.pl Outdated
Comment thread tests/runtests.pl Outdated
Co-authored-by: Viktor Szakats <vszakats@users.noreply.github.com>
@mihaigmarin

Copy link
Copy Markdown
Contributor Author

@vszakats Thank you for your review. I have applied the changes you suggested

@vszakats

Copy link
Copy Markdown
Member

Thanks for the fixes @mihaigmarin.

One more fix is necessary, CI reports mixed newlines:

tests/runtests.pl: content: has mixed EOL types
tests/runtests.pl: content: must use LF EOL for this file type

Signed-off-by: Mihai-Gabriel Marin <mihaigmarin@gmail.com>
@mihaigmarin

Copy link
Copy Markdown
Contributor Author

No problem @vszakats, it is my pleasure.
I have removed the mixed EOL.
I think they were added when applied the suggested changed directly on github web interface 🤔

Comment thread tests/runtests.pl Outdated
my %singletest_logs; # log messages while in singletest array ref by runner
my $singletest_bufferedrunner; # runner ID which is buffering logs
my %runnerids; # runner IDs by number
my %runnernums; # runner numbers by IDs

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.

Maybe add a comment to make it explicit that this is the same data as %runnerids, just inverted.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done

Comment thread tests/runtests.pl
Co-authored-by: Dan Fandrich <dan@coneharvesters.com>
Signed-off-by: Mihai-Gabriel Marin <mihaigmarin@gmail.com>
@bagder bagder closed this in 5bb0bea Sep 3, 2026
@bagder

bagder commented Sep 3, 2026

Copy link
Copy Markdown
Member

Thanks!

@mihaigmarin

Copy link
Copy Markdown
Contributor Author

My pleasure

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

Development

Successfully merging this pull request may close these issues.

5 participants