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 "last step wins" error #132

Merged
merged 4 commits into from
Dec 3, 2023
Merged

Conversation

wadells
Copy link
Contributor

@wadells wadells commented Nov 28, 2023

This PR fixes #133, a regression introduced in #127.

Specifically: when a job had multiple steps, the runAssertions check on the last step overwrote the jobHasError value from all previous steps. This will cause the following job to pass, even though the initial actions/checkout is unpinned.

on:
  push:

name: Continuous Integration

jobs:
  check-github-actions:
    name: Check GitHub Actions
    runs-on: ubuntu-latest
    steps:
      - name: Checkout code
        uses: actions/checkout@v4
      - name: Ensure SHA pinned actions
        uses: zgosalvez/github-actions-ensure-sha-pinned-actions@b35f285b9bb7e80de0967367cee66d3b6d50ceca # v3.0.1

Testing

I added a job to the unpinned stubs that simulates the above workflow. I don't have javascript/jest setup locally, so I'm counting on CI to let me know if I'm missing anything. You're welcome to edit this PR (or close it and do it a better way) if this isn't a suitable fix.

Whether the job fails or not was being overwritten by the last step in
the job. If the last step is pinned, this would cause the check to
pass, even if an earlier step was unpinned. Now we have a test case that
exercises this functionality.
@wadells wadells changed the title Fix last step wins error Fix "last step wins" error Nov 28, 2023
Previously this logic only failed the job if the last step had an error.
@zgosalvez zgosalvez enabled auto-merge (squash) December 3, 2023 02:23
@zgosalvez zgosalvez disabled auto-merge December 3, 2023 02:43
@zgosalvez zgosalvez merged commit b1b635d into zgosalvez:main Dec 3, 2023
1 of 2 checks passed
@zgosalvez
Copy link
Owner

Thank you so much for contributing this fix! 🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Jobs with multiple steps will not fail if the last step is pinned
2 participants