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

Fix or work around broken tests in new analysis #14729

Closed
6 tasks done
cameel opened this issue Dec 12, 2023 · 2 comments · Fixed by #14731
Closed
6 tasks done

Fix or work around broken tests in new analysis #14729

cameel opened this issue Dec 12, 2023 · 2 comments · Fixed by #14731

Comments

@cameel
Copy link
Member

cameel commented Dec 12, 2023

See also #14510 (comment)

Remaining fixes before we can have green CI and merge the prototype:

@ekpyron
Copy link
Member

ekpyron commented Dec 12, 2023

We definitely should not waste effort on #13925. Why doesn't the filter in prepare_report.py already cover these cases?

@cameel
Copy link
Member Author

cameel commented Dec 13, 2023

I tried hard to avoid prepare_report.py having to parse CLI output and fish ICEs out of it. Instead it just checks whether it can find the artifacts. No artifacts == error. The filter is only necessary for Standard JSON to ignore inconsistent artifacts.

But actually, turns out that this was not the cause. The script had a bug - it was not looking at the exit code, which for CLI is another indication that something went wrong. The script originally only handled Standard JSON and there the exit code does not matter. When I was adding CLI support later, I must have forgotten to add a check for that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants