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][3PP License Check] Detect when dash-licenses exits with an internal error #12545

Merged
merged 1 commit into from
May 18, 2023

Conversation

marcdumais-work
Copy link
Contributor

What it does

This commit adapts the check_3pp_licenses.js script to handle new "127" exit code from dash-licenses. This follows us noticing that it was possible for the script to not detect that dash-licenses had encountered an internal error, not related to the 3PPs licenses, that voided the run's results. We will now detect such cases and appropriately exit with an error code, so that if this happens in CI, the job will fail.

See dash-licenses issue:
eclipse-dash/dash-licenses#236

and Pull Request:
eclipse-dash/dash-licenses#237

How to test

Check that you can successfully still run yarn license:check locally. I guess it would be possible to use a mock dash-licenses that exits with code 127 could be used to test the change, but it may not be necessary.

Review checklist

Reminder for reviewers

…nal error

This commit adapts the `check_3pp_licenses.js` script to handle new "127" exit code
from dash-licenses. This follows us noticing that it was possible for the script
to not detect that `dash-licenses` had encountered an internal error, not related
to the 3PPs licenses, that voided the run's results. We will now detect such cases
and appropriately exit with an error code, so that if this happens in CI, the job
will fail.

See dash-licenses issue:
eclipse-dash/dash-licenses#236

and Pull Request:
eclipse-dash/dash-licenses#237

Signed-off-by: Marc Dumais <marc.dumais@ericsson.com>
@marcdumais-work marcdumais-work added the ci issues related to CI / tests label May 18, 2023
Copy link
Member

@vince-fugnitto vince-fugnitto left a comment

Choose a reason for hiding this comment

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

LGTM 👍
I did a run where errors are expected from dash and they correctly were reported (vince-fugnitto#18).

@marcdumais-work marcdumais-work merged commit 0c634ea into master May 18, 2023
@marcdumais-work marcdumais-work deleted the support-dash-licenses-internal-error branch May 18, 2023 17:35
@github-actions github-actions bot added this to the 1.38.0 milestone May 18, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ci issues related to CI / tests
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

2 participants