Skip to content

Conversation

@jcfr
Copy link
Contributor

@jcfr jcfr commented Aug 26, 2025

This pull request enhances the test runner script with two main improvements:

  1. Refactors the script to introduce a run() function that centralizes the logic for executing compiler commands, capturing their output, and returning results.
  2. Adds checks to ensure that required compilers (clang, gcc) are present before running tests.

@jcfr jcfr force-pushed the improve-test-runner branch from 86f3774 to 3505c8f Compare August 26, 2025 04:59
@jcfr jcfr force-pushed the improve-test-runner branch 3 times, most recently from dc55734 to 129b151 Compare August 26, 2025 08:58
@jcfr jcfr force-pushed the improve-test-runner branch 2 times, most recently from c6a0970 to 51a3803 Compare August 26, 2025 16:30
@jcfr
Copy link
Contributor Author

jcfr commented Aug 26, 2025

Thanks for the taking the time to review 🙏

All the review comments have been addressed. Once the GitHub workflow have been enabled and the test pass, I believe this will be ready for integration 🚀

cc: @danmar @firewave

@jcfr
Copy link
Contributor Author

jcfr commented Aug 27, 2025

For additional context, the initial goal of this pull request was to allow me to understand the missing prerequisites.

The proposed improvements fulfill this. This is now ready for integration 🙏

@jcfr jcfr force-pushed the improve-test-runner branch from 2f1962a to 8203180 Compare August 28, 2025 18:10
@hjmjohnson
Copy link

@jcfr Thank you for making this MUCH easier to understand.

@firewave
Copy link
Collaborator

For additional context, the initial goal of this pull request was to allow me to understand the missing prerequisites.

Yeah - it is (was) a bit of a mess. I just started digging into it in the past few days myself.

@jcfr jcfr force-pushed the improve-test-runner branch from 8203180 to 2bc5572 Compare August 29, 2025 14:41
jcfr added 4 commits August 29, 2025 12:00
This change refactors the test runner script by adding a `run()` function
for executing compiler commands, capturing their output, and returning the
results.
This change updates the test runner script to check for the presence of
required compilers (clang, gcc) before running tests.
Suggested-by: Oliver Stöneberg <firewave@users.noreply.github.com>
…s handling

Suggested-by: Oliver Stöneberg <firewave@users.noreply.github.com>
jcfr added 3 commits August 29, 2025 12:00
Suggested-by: Oliver Stöneberg <firewave@users.noreply.github.com>
`from __future__ import annotations` is useful in importable packages to
enable postponed evaluation of annotations. However, in a standalone
script like `run-tests.py`, it adds no value.
@jcfr jcfr force-pushed the improve-test-runner branch from 2bc5572 to 89e6b31 Compare August 29, 2025 16:00
Copy link
Collaborator

@firewave firewave left a comment

Choose a reason for hiding this comment

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

LGTM. The stuff I wanted to check will need to be redone anyways, so no need to further look into that.

@firewave firewave merged commit 37fa4f4 into danmar:master Aug 30, 2025
14 checks passed
@jcfr jcfr deleted the improve-test-runner branch August 30, 2025 21:54
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.

4 participants