Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fail loudly if new benchmark has no output. #135

Merged
merged 1 commit into from
Apr 18, 2024

Conversation

n8xm
Copy link
Contributor

@n8xm n8xm commented Apr 17, 2024

When --make_benchmarks is on, a misconfigured / buggy test program might fail to produce any output. If this happens, the regression testing script will continue executing and eventually crash when it can't find the .status file that it is expecting to be there, creating an unintuitive backtrace that does not make clear the root cause of the problem:

Traceback (most recent call last):
  File "./regtest.py", line 1317, in <module>
    n = test_suite(sys.argv[1:]) File "./regtest.py", line 1245, in test_suite
    test_list, args.input_file[0])
  File "/home/nathan/Code/regression_testing/test_report.py", line 954, in report_this_test_run with 
    open(benchStatusFile) as bf:
FileNotFoundError: [Errno 2] No such file or directory: 'TestName.status' 

This PR fixes the above by failing with a clear error message as soon as it can be determined that output from the test program cannot be found.

When --make_benchmarks is on, a misconfigured / buggy test program might
fail to produce any output. If this happens, the regression testing
script will continue executing and eventually crash when it can't find
the .status file that it is expecting to be there. This produces an
unintuitive backtrace that does not make clear the root cause of the
problem.

This PR fixes the above by failing with a clear error message as soon as
it can be determined that output from the test program cannot be found.
@n8xm n8xm changed the title error no test output Fail loudly if new benchmark has no output. Apr 17, 2024
@zingale zingale merged commit c17887e into AMReX-Codes:main Apr 18, 2024
1 check failed
@ax3l
Copy link
Member

ax3l commented Apr 22, 2024

Oh no, this broke WarpX #136.

Can we make this optional?

@zingale
Copy link
Member

zingale commented Apr 22, 2024

I'm okay with making it optional -- @n8xm are you okay with making this off by default and enabled with a runtime parameter?

@zingale
Copy link
Member

zingale commented Jun 16, 2024

this breaks our test suites

I am going to make it optional

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.

3 participants