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

CI Remove empty test reference files #1164

Merged
merged 2 commits into from
Jul 22, 2024

Conversation

jarzec
Copy link
Contributor

@jarzec jarzec commented Jul 15, 2024

  • Update the run-tests.sh.
    • Handle empty generated files as correct without the need for committing them.
    • Remove such files generated by the test.
    • Report empty reference files as errors to maintain the status quo.
  • Add a rm-empty-files.sh utility script for removing all empty files in current directory.
  • Remove all empty reference test files (>1500 empty files less in the repository).

Note: This PR is one step towards improvements of the kind proposed in #1137. It should make further improvement simpler.
An iterative approach should be easier to handle.

@jarzec
Copy link
Contributor Author

jarzec commented Jul 15, 2024

The test failures will be fixed by #1165.

@jarzec jarzec force-pushed the ci-remove-empty-files branch from fc3af56 to dfdee88 Compare July 16, 2024 07:36
@jarzec jarzec force-pushed the ci-remove-empty-files branch from dfdee88 to d9e99c8 Compare July 22, 2024 19:25
@hsutter hsutter merged commit 1fdc1e8 into hsutter:main Jul 22, 2024
16 of 28 checks passed
@hsutter
Copy link
Owner

hsutter commented Jul 22, 2024

Thanks!

@jarzec
Copy link
Contributor Author

jarzec commented Jul 22, 2024

@hsutter My last update of this PR was missing some changes from main and is now causing test failures.
I am working on a fix in #1176 - it should become all green soon.

@hsutter
Copy link
Owner

hsutter commented Jul 25, 2024

Hmm... one thing I meant to ask before merging this (but forgot, sorry) was:

Early on, the reason I was storing files even if empty was because otherwise I couldn't distinguish between "test ran successfully and there was no output" and "test crashed and failed to generate any output."

Are we still going to catch the "empty" vs "crashed" case somehow so that we don't start getting silent regressions due to crashes for test cases that would have empty output anyway?

@jarzec
Copy link
Contributor Author

jarzec commented Jul 25, 2024

That is a good point. The test script currently silently removes the generated empty file. I will add a check that the file actually got created.

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