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

Provide an option to fail the action itself #265

Closed
laeubi opened this issue May 10, 2022 · 16 comments
Closed

Provide an option to fail the action itself #265

laeubi opened this issue May 10, 2022 · 16 comments

Comments

@laeubi
Copy link

laeubi commented May 10, 2022

We have an action that publish the junit tests in a separate step:

https://github.com/eclipse-platform/eclipse.platform.swt/actions/workflows/junit.yml

but for the master build there are obviously nothing to publish as there is no PR so currently one gets the following output:

2022-05-09 22:16:03 +0000 - publish-unit-test-results - INFO - reading artifacts/**/*.xml
2022-05-09 22:16:03 +0000 - publish - INFO - publishing failure results for commit 2eac2956b78d8d35ec514a603e64f3c649203eb6
2022-05-09 22:16:04 +0000 - publish - INFO - created check https://github.com/eclipse-platform/eclipse.platform.swt/runs/6360507060
2022-05-09 22:16:05 +0000 - publish - INFO - there is no pull request for commit 2eac2956b78d8d35ec514a603e64f3c649203eb6

Example: https://github.com/eclipse-platform/eclipse.platform.swt/runs/6360504104

To create a status badge that shows if tests on the master branch are all successful it would be useful to have an option that the action itself should fail if

  1. there is no PR to publish the data
  2. there are testfailures
@EnricoMi
Copy link
Owner

That is an interesting an particular finding. Let's summarize the situation:

The action creates a check run on that commit on master, which has failure state:
grafik

This is associated with the commit:
grafik

So everything looks good for GitHub to consider master branch not passing.

However, those badges are per workflow, and the workflow did pass:
grafik
grafik

The check run is not part of the workflow, though it is in some views shown there. This is related to #12.

Given the publish action runs in a separate workflow on a workflow_run event, a badge on that workflow would be what you want. But that workflow cannot distinguish between branches.

@EnricoMi
Copy link
Owner

EnricoMi commented May 10, 2022

I would expect the Build with Maven step in your Verify * Jobs to fail on unit test failures. Why isn't this the case?

This would achieve a job and workflow failure, turning the batch to failure state.

@laeubi
Copy link
Author

laeubi commented May 10, 2022

However, those badges are per workflow, and the workflow did pass:

Correct, for sure it would be better if everything always passes but we have cases where we need to accept failing tests for a while (e.g. there are os changes we need to adapt) so a red (or yellow) badge would be good.

I would expect the Build with Maven step in your Verify * Jobs to fail on unit test failures. Why isn't this the case?

We disabled that maven fails but instead just runs all the tests and later on the verify check post the result of the unit test results, that way one could see for example if a test only fails on a certain platform configuration or java even thozgh the compile itself succeeds.

This would achieve a job and workflow failure, turning the batch to failure state.

The split is manly done due to https://github.com/EnricoMi/publish-unit-test-result-action#support-fork-repositories-and-dependabot-branches as we use forks for all code changes.

@EnricoMi
Copy link
Owner

EnricoMi commented May 10, 2022

I don't get it. You could achieve all this by:

  • having each verify job fail on test failures (not immediately, but --fail-at-end)
  • always upload test results
  • always publish test results

eclipse-platform/eclipse.platform.swt@d470bcf...EnricoMi:416507e528484d35df3ab1354d870f11fc6ac7b8

.github/workflows/maven.yml:

         -DforkCount=1
         -Dcompare-version-with-baselines.skip=false
         -Dmaven.compiler.failOnWarning=true
-        -Dmaven.test.failure.ignore=true
+        --fail-at-end
         clean verify
        working-directory: eclipse.platform.swt
     - name: Upload Test Results for ${{ matrix.config.name }} / Java-${{ matrix.java }}
       uses: actions/upload-artifact@v3
+      if: always()
       with:
         name: test-results-${{ matrix.config.native }}-java${{ matrix.java }}
         if-no-files-found: error

.github/workflows/junit.yml:

  unit-test-results:
     name: Unit Test Results
     runs-on: ubuntu-latest
-    if: github.event.workflow_run.conclusion == 'success'
+    if: github.event.workflow_run.conclusion != 'skipped'

     steps:
       - name: Download and Extract Artifacts

Then your master badge turns red:
SWT Matrix Build

And the CI tells you which platform or java fails:
grafik

@laeubi
Copy link
Author

laeubi commented May 10, 2022

I don't get it. You could achieve all this by:

I want a green badge if compiling succeeds on all platforms and a another if all test passed (or a red/yellow/... if any tests currently fail of course) otherwise one could still get a false impression (without looking at the build output) that there are only test failures even if some modules fail with compile error (as -fae do not work for only tests).

@EnricoMi
Copy link
Owner

Right, then I would suggest two workflows, one building without bothering to run tests for your green build badge, and one thoroughly testing for your test-failure badge. You could have the former workflow only run on master.

@laeubi
Copy link
Author

laeubi commented May 10, 2022

But I already have two workflows right now ... ;-)

And if I want to support PRs I then need three of them... that's why I thought it might be good to have an option that the publish can mark the current workflow as a failure! e.g. if I follow the "show this check" e.g. https://github.com/eclipse-platform/eclipse.platform.swt/runs/6372153691 then the workflow is marked with a red-x so I assume it already tells github that are something wrong, but it is not bubbled up to the workflow run... so it works already for PRs as expected but just seem to be marked as "good" when there is no PR...

@EnricoMi
Copy link
Owner

Even if the publish job would fail, this would not make the master badge fail as it runs as workflow_run event. The publish action would have to run along with the build workflow, which then stops working forks.

@laeubi
Copy link
Author

laeubi commented May 10, 2022

But why do I see then runs of the workflow with master branch? That was just the 'trigger' for me to ask for that feature:
https://github.com/eclipse-platform/eclipse.platform.swt/actions/workflows/junit.yml?query=branch%3Amaster

also these runs seem to do something or do I get the log output wrong here?:

022-05-10 15:29:59 +0000 - publish-unit-test-results - INFO - reading artifacts/**/*.xml
2022-05-10 15:30:00 +0000 - publish - INFO - publishing failure results for commit d470bcf8a0e1acb9bbfb014a069019a1237b887e
2022-05-10 15:30:01 +0000 - publish - INFO - created check https://github.com/eclipse-platform/eclipse.platform.swt/runs/6373026177
2022-05-10 15:30:02 +0000 - publish - INFO - there is no pull request for commit d470bcf8a0e1acb9bbfb014a069019a1237b887e

@EnricoMi
Copy link
Owner

The workflow_run event always runs on master, even if triggered from a branch or fork.

That log tells you that the results were published as check run https://github.com/eclipse-platform/eclipse.platform.swt/runs/6373026177 associated with commit d470bcf8 on master.

@EnricoMi
Copy link
Owner

I just found that there already is a way to can make the publish job fail.

Simply evaluate the JSON output like this:

       - name: Publish Unit Test Results
+        id: test-results
         uses: EnricoMi/publish-unit-test-result-action@v1
         with:
           commit: ${{ github.event.workflow_run.head_sha }}
           files: "artifacts/**/*.xml"
+      - name: Fail on test failure
+        if: fromJSON( steps.test-results.outputs.json ).conclusion != 'success'
+        run: |
+          echo "Conclusion is ${{ fromJSON( steps.test-results.outputs.json ).conclusion }}"
+          false

Curious if this works with workflow_run.

@EnricoMi
Copy link
Owner

EnricoMi commented May 10, 2022

With two build workflows this definitively works fine:
SWT Matrix Build SWT Matrix Build and Test

eclipse-platform/eclipse.platform.swt@master...EnricoMi:master

As I said, the problem with the publish workflow badge is that it cannot distinguish between the branch that triggered it. Results from PRs will be accounted for master as well.

@laeubi
Copy link
Author

laeubi commented May 10, 2022

Results from PRs will be accounted for master as well.

Okay I was not aware of that, than probably this request do not make much sense and could be closed, thanks for the detailed insights!

@laeubi laeubi closed this as completed May 10, 2022
@EnricoMi
Copy link
Owner

Alternatively, you could create a badge from the test results, upload to gist and use that to flag your master tests read when they fail: https://github.com/EnricoMi/publish-unit-test-result-action/tree/v1.36#create-a-badge-from-test-results

@laeubi
Copy link
Author

laeubi commented May 19, 2022

That looks really cool!

@laeubi
Copy link
Author

laeubi commented Jun 8, 2022

@EnricoMi I have tried your suggestion but can't get it to work, it always complains about non existing file:

https://github.com/laeubi/eclipse.platform.swt/runs/6790512790

I have tried to create the file with some dummy content:
https://gist.github.com/laeubi/f58c7b953f3b46c432b7ec9d59d70a6b

so it seems I'm missing something here...

Edit: Everything works, I just messed up gist ids and filenames...

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

No branches or pull requests

2 participants