Skip to content

roottest: avoid generated unneeded pcm files #19324

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

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

pcanal
Copy link
Member

@pcanal pcanal commented Jul 9, 2025

The tests are checking if the dictionary source files can compile.

Having the pcm files around can affect the other tests in the same directory, for example in case those
other test runs before the pcm has been rebuild (and it depend on something that has already been rebuild).

@pcanal pcanal requested a review from vepadulano July 9, 2025 21:21
@pcanal pcanal requested a review from bellenot as a code owner July 9, 2025 21:21
Copy link

github-actions bot commented Jul 9, 2025

Test Results

    21 files      21 suites   3d 8h 55m 17s ⏱️
 3 215 tests  3 215 ✅ 0 💤 0 ❌
65 798 runs  65 798 ✅ 0 💤 0 ❌

Results for commit 58d7587.

♻️ This comment has been updated with latest results.

@pcanal pcanal closed this Jul 14, 2025
@pcanal pcanal reopened this Jul 14, 2025
@pcanal pcanal requested a review from dpiparo July 15, 2025 21:19
pcanal added 2 commits July 20, 2025 14:20
The tests are checking if the dictionary source files can compile.
Having the pcm files around can affect the other tests in the same directory, for example in case those
other test runs before the pcm has been rebuild (and it depend on something that has already been rebuild).
@pcanal pcanal force-pushed the roottest-no-pcm branch from 213f5e7 to d22bb92 Compare July 21, 2025 22:21
@pcanal pcanal marked this pull request as draft July 22, 2025 00:31
@pcanal pcanal added the clean build Ask CI to do non-incremental build on PR label Jul 22, 2025
@pcanal pcanal closed this Jul 22, 2025
@pcanal pcanal reopened this Jul 22, 2025
@pcanal pcanal marked this pull request as ready for review July 23, 2025 13:23
@pcanal
Copy link
Member Author

pcanal commented Jul 23, 2025

@dpiparo @vepadulano This is now ready for review. It works well but requires a clean build (to get rid of the the stale-no-longer produced pcm files).

Copy link
Member

@vepadulano vepadulano left a comment

Choose a reason for hiding this comment

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

Mostly good, but I believe we should address the CI workflow issue separately

@@ -583,3 +583,19 @@ jobs:
- name: ccache info (post)
run: |
ccache -s || true

event_file:
Copy link
Member

Choose a reason for hiding this comment

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

Looks like spurious changes?

Copy link
Member

Choose a reason for hiding this comment

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

Ok I now saw the commit message mentioning

This seems to prevent the display of error logs in the web interface.

I believe this should be tackled in a separate PR

@pcanal
Copy link
Member Author

pcanal commented Jul 23, 2025

Mostly good, but I believe we should address the CI workflow issue separately

Yes, it actually was.

@vepadulano
Copy link
Member

Yes, it actually was.

Ok, does it mean that the third commit of this PR can be removed?

@pcanal pcanal force-pushed the roottest-no-pcm branch from d22bb92 to fdea796 Compare July 23, 2025 17:11
@pcanal pcanal requested a review from vepadulano July 23, 2025 17:12
@pcanal pcanal force-pushed the roottest-no-pcm branch 2 times, most recently from cb8fc47 to b60aac6 Compare July 23, 2025 17:57
@pcanal pcanal force-pushed the roottest-no-pcm branch from b60aac6 to 58d7587 Compare July 23, 2025 20:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clean build Ask CI to do non-incremental build on PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants