Skip to content

Fix log-format reporter ignoring install errors#25961

Merged
haampie merged 2 commits intospack:developfrom
Jordan474:fix-log-format-ignoring-errors
Nov 9, 2021
Merged

Fix log-format reporter ignoring install errors#25961
haampie merged 2 commits intospack:developfrom
Jordan474:fix-log-format-ignoring-errors

Conversation

@Jordan474
Copy link
Copy Markdown
Contributor

@Jordan474 Jordan474 commented Sep 15, 2021 •
edited
Loading

When running spack install --log-format junit|cdash ..., install errors were ignored. This made spack continue building dependents of failed install, ignoring --fail-fast, and exit 0 at the end.

I found it was the InfoCollector class not forwarding exceptions after gathering them for report.

Slight behavior change: (but for the best) because dependents of a failed install are now correctly skipped (like without --log-format), they might not be reported the same as before in the report file (skipped build is not an error).

Fixes #5378

@Jordan474 Jordan474 marked this pull request as draft September 15, 2021 09:29
@spackbot-app spackbot-app bot added the tests General test capability(ies) label Sep 15, 2021
@Jordan474 Jordan474 force-pushed the fix-log-format-ignoring-errors branch from cbf0fbd to 3d9e5e7 Compare September 15, 2021 09:36
@haampie
Copy link
Copy Markdown
Member

haampie commented Sep 15, 2021

Thanks for fixing this 4-digit issue number 😅

Can you also fix the TODO's from the failing tests:

        # TODO: Why does junit output capture appear to swallow the exception
        # TODO: as evidenced by the two failing packages getting tagged as
        # TODO: installed?
        with tmpdir.as_cwd():
>           install('--log-format=junit', '--log-file=test.xml', 'libdwarf')

@Jordan474 Jordan474 force-pushed the fix-log-format-ignoring-errors branch 4 times, most recently from 2ccf66a to aaab317 Compare September 15, 2021 15:15
@Jordan474 Jordan474 marked this pull request as ready for review September 16, 2021 07:01
@bernhardkaindl
Copy link
Copy Markdown
Contributor

bernhardkaindl commented Oct 7, 2021

@Jordan474 ping for question by haampie:

Thanks for fixing this 4-digit issue number sweat_smile

Can you also fix the TODO's from the failing tests:

        # TODO: Why does junit output capture appear to swallow the exception
        # TODO: as evidenced by the two failing packages getting tagged as
        # TODO: installed?
        with tmpdir.as_cwd():
>           install('--log-format=junit', '--log-file=test.xml', 'libdwarf')

@Jordan474
Copy link
Copy Markdown
Contributor Author

Jordan474 commented Oct 7, 2021

@haampie @bernhardkaindl Yes, this fixed the TODO (I removed the comment): junit does not swallow the exception anymore, and the command fails as expected (I added the assert to test that).

@bernhardkaindl bernhardkaindl self-assigned this Oct 7, 2021
@bernhardkaindl bernhardkaindl requested a review from haampie October 7, 2021 21:15
haampie
haampie previously approved these changes Nov 9, 2021
View reviewed changes
@haampie haampie closed this Nov 9, 2021
@haampie haampie reopened this Nov 9, 2021
@haampie haampie mentioned this pull request Nov 9, 2021
Merged
24 tasks
@haampie
Copy link
Copy Markdown
Member

haampie commented Nov 9, 2021

@spackbot run pipeline

@spackbot-app
Copy link
Copy Markdown

spackbot-app bot commented Nov 9, 2021

I've started that pipeline for you!

Jordan Galby added 2 commits November 9, 2021 13:39
@haampie
Fix log-format reporter ignoring install errors
86596cc 
When running `spack install --log-format junit|cdash ...`, install
errors were ignored. This made spack continue building dependents of
failed install, ignoring `--fail-fast`, and exit 0 at the end.

Fixes spack#5378
@haampie
Fix install log-format tests
acf2b23 
Now that spack install --log-format properly reports failure (just like
when we are not using a --log-format), tests must be fixed.
@haampie haampie force-pushed the fix-log-format-ignoring-errors branch from aaab317 to acf2b23 Compare November 9, 2021 12:39
@haampie
Copy link
Copy Markdown
Member

haampie commented Nov 9, 2021 •
edited
Loading

Rebased it to avoid a lot of redundant recompilation in gitlab

@haampie haampie enabled auto-merge (squash) November 9, 2021 12:40
@haampie haampie merged commit 6e9c0a8 into spack:develop Nov 9, 2021
haampie pushed a commit that referenced this pull request Nov 9, 2021
@Jordan474 @haampie
Fix log-format reporter ignoring install errors (#25961)
928538e 
When running `spack install --log-format junit|cdash ...`, install
errors were ignored. This made spack continue building dependents of
failed install, ignoring `--fail-fast`, and exit 0 at the end.
haampie pushed a commit that referenced this pull request Nov 16, 2021
@Jordan474 @haampie
Fix log-format reporter ignoring install errors (#25961)
7d080c3 
When running `spack install --log-format junit|cdash ...`, install
errors were ignored. This made spack continue building dependents of
failed install, ignoring `--fail-fast`, and exit 0 at the end.
alalazo pushed a commit that referenced this pull request Dec 23, 2021
@Jordan474 @alalazo
Fix log-format reporter ignoring install errors (#25961)
7c6b253 
When running `spack install --log-format junit|cdash ...`, install
errors were ignored. This made spack continue building dependents of
failed install, ignoring `--fail-fast`, and exit 0 at the end.
capitalaslash pushed a commit to capitalaslash/spack that referenced this pull request Aug 30, 2022
@Jordan474 @capitalaslash
Fix log-format reporter ignoring install errors (spack#25961)
47742d9 
When running `spack install --log-format junit|cdash ...`, install
errors were ignored. This made spack continue building dependents of
failed install, ignoring `--fail-fast`, and exit 0 at the end.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@haampie haampie haampie approved these changes

@becker33 becker33 Awaiting requested review from becker33

Assignees

@bernhardkaindl bernhardkaindl

Labels

tests General test capability(ies)

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Adding --log-format to spack install alway returns exit code 0

3 participants

@Jordan474 @haampie @bernhardkaindl