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

cypress-io/github-action does not distinguish steps.<step_id>.conclusion from steps.<step_id>.outcome #964

Closed
ricardo-dematos opened this issue Jul 14, 2023 · 14 comments

Comments

@ricardo-dematos
Copy link

Given the following configuration, when one of the tests fails both steps.cypress.conclusion and steps.cypress.outcome report failure , thus contradicting the documentation.

      - name: 'Run end-to-end tests'
        id: cypress
        timeout-minutes: 15  # must be adjusted with e2e tests progress
        uses: cypress-io/github-action@v5  # see https://github.com/cypress-io/github-action/issues/620
        env:
          ELECTRON_EXTRA_LAUNCH_ARGS: '--disable-gpu'  # see https://github.com/cypress-io/cypress/issues/25357
        with:
          working-directory: 'e2e-tests'
          cache-key: "node-cache-${{ runner.os }}-npm-${{ hashFiles('e2e-tests/package-lock.json') }}"  # see https://github.com/cypress-io/github-action/pull/24
          command: 'npx cypress run --e2e --browser ${{ inputs.browser }} --headless --config video=false --env host="${{ inputs.deployment_url }}",username="${{ inputs.deployment_username }}",password="${{ inputs.deployment_password }}"'  # see https://github.com/cypress-io/github-action/issues/489

      - name: 'Print previous step outcome/conclusion'
        id: print
        run: |
          echo "outcome: ${{ steps.cypress.outcome }}"
          echo "conclusion: ${{ steps.cypress.conclusion }}"
        shell: bash

As long cypress-io/github-action completes with success, it should report steps.cypress.conclusion=success .

This may be related to the use of a custom command.

@ricardo-dematos
Copy link
Author

It should also be noted that the use of a custom command produces an error message:

image

It seems that the exit code matches the number of failed tests:

image
image

@MikeMcC399
Copy link
Collaborator

@ricardo-dematos

Given the following configuration, when one of the tests fails both steps.cypress.conclusion and steps.cypress.outcome report failure , thus contradicting the documentation.

If you think there is a bug in github-action in this respect, please provide a simple reproducible example. However please note my comments:

I cannot see any use of continue-on-error in your example. That would be the only reason that conclusion and outcome would differ. It is probably not a good idea anyway to use continue-on-error because that could make the Cypress test look like it passed even it if failed.

The example workflow you posted will not display the step outcomes after an error in the previous step in the job occurs. You could add
if: always()
to correct that.

      - name: 'Print previous step outcome/conclusion'
        if: always()

@ricardo-dematos
Copy link
Author

If you think there is a bug in github-action in this respect, please provide a simple reproducible example.

The reported observation comes from implementing/testing a workflow to run our Cypress/Cucumber E2E tests.

After running the tests, we want to generate a Cucumber HTML report and then upload it together with the screenshots. This must happen regardless of the tests results, has long the step concludes with success.

      - name: 'Set up steps'
        id: setup
        run: |
          echo "repository=$( echo "${{ github.event.repository.full_name || github.repository }}" )" >> $GITHUB_OUTPUT
          echo "repository_owner=$( echo "${{ github.event.repository.owner.name || github.repository_owner }}" )" >> $GITHUB_OUTPUT
          echo "repository_name=$( echo "${{ github.event.repository.name }}" )" >> $GITHUB_OUTPUT
          echo "branch_ref=$( echo "${{ github.event.ref || github.ref }}" )" >> $GITHUB_OUTPUT
          echo "branch_name=$( echo "${{ github.event.ref_name || github.ref_name }}" )" >> $GITHUB_OUTPUT
          if [[ ("${{ github.event_name }}" == "workflow_dispatch") && ("${{ github.workflow }}" == "End-to-End Tests") ]]; then
            echo "::add-mask::$( cat $GITHUB_EVENT_PATH | jq -e -M -r '.inputs?."deployment-url"?' | sed --regexp-extended --expression 's/\/$//g' )"
            echo "::add-mask::$( cat $GITHUB_EVENT_PATH | jq -e -M -r '.inputs?."deployment-username"?' )"
            echo "::add-mask::$( cat $GITHUB_EVENT_PATH | jq -e -M -r '.inputs?."deployment-password"?' )"
            echo "deployment_url=$( cat $GITHUB_EVENT_PATH | jq -e -M -r '.inputs?."deployment-url"?' | sed --regexp-extended --expression 's/\/$//g' )" >> $GITHUB_OUTPUT
            echo "deployment_username=$( cat $GITHUB_EVENT_PATH | jq -e -M -r '.inputs?."deployment-username"?' )" >> $GITHUB_OUTPUT
            echo "deployment_password=$( cat $GITHUB_EVENT_PATH | jq -e -M -r '.inputs?."deployment-password"?' )" >> $GITHUB_OUTPUT
            echo "browser=$( cat $GITHUB_EVENT_PATH | jq -e -M -r '.inputs?."browser"?' )" >> $GITHUB_OUTPUT
          else
            echo "deployment_url=$( echo "${{ secrets.E2E_TESTS_DEPLOYMENT_URL }}" )" >> $GITHUB_OUTPUT
            echo "deployment_username=$( echo "${{ secrets.E2E_TESTS_DEPLOYMENT_USERNAME }}-${{ github.event.inputs.browser || inputs.browser }}" )" >> $GITHUB_OUTPUT
            echo "deployment_password=$( echo "${{ secrets.E2E_TESTS_DEPLOYMENT_PASSWORD }}" )" >> $GITHUB_OUTPUT
            echo "browser=$( echo "${{ github.event.inputs.browser || inputs.browser }}" )" >> $GITHUB_OUTPUT
          fi
        shell: bash

      - name: 'Check out repository'
        id: checkout
        if: ${{ (success() || failure()) && (steps.setup.outcome == 'success') }}
        uses: actions/checkout@v3
        with:
          ref: '${{ steps.setup.outputs.branch_ref }}'

      - name: 'Set up node.js'
        id: node
        if: ${{ (success() || failure()) && (steps.checkout.outcome == 'success') }}
        uses: valispace/actions/setup-node@master
        with:
          node-version: ${{ vars.NODE_VERSION }}
          node-dependencies-file: 'e2e-tests/package-lock.json'
          node-install-dependencies: true

      - name: 'Install dependencies'  # see https://docs.cypress.io/guides/continuous-integration/introduction#Dependencies
        id: dependencies
        if: ${{ (success() || failure()) && (steps.node.outcome == 'success') }}
        run: 'sudo apt-get --quiet --no-install-recommends --assume-yes install libgtk2.0-0 libnotify-dev'
        shell: 'bash'

      - name: 'Run end-to-end tests'
        id: cypress
        if: ${{ (success() || failure()) && (steps.dependencies.outcome == 'success') }}
        timeout-minutes: 15  # must be adjusted with e2e tests progress
        uses: cypress-io/github-action@v5  # see https://github.com/cypress-io/github-action/issues/620
        env:
          ELECTRON_EXTRA_LAUNCH_ARGS: '--disable-gpu'  # see https://github.com/cypress-io/cypress/issues/25357
        with:
          working-directory: 'e2e-tests'
          cache-key: "node-cache-${{ runner.os }}-npm-${{ hashFiles('e2e-tests/package-lock.json') }}"  # see https://github.com/cypress-io/github-action/pull/24
          command: 'npx cypress run --e2e --browser ${{ steps.setup.outputs.browser }} --headless --config video=false --env host="${{ steps.setup.outputs.deployment_url }}",username="${{ steps.setup.outputs.deployment_username }}",password="${{ steps.setup.outputs.deployment_password }}"'  # see https://github.com/cypress-io/github-action/issues/489
          browser: '${{ steps.setup.outputs.browser }}'
          headed: false
          config: 'video=false'  # see https://github.com/cypress-io/github-action/issues/483
          env: 'host="${{ steps.setup.outputs.deployment_url }}",username="${{ steps.setup.outputs.deployment_username }}",password="${{ steps.setup.outputs.deployment_password }}"'

      - name: 'Generate end-to-end tests report'
        # if: ${{ (success() || failure()) && (steps.cypress.conclusion == 'success') }}  # see https://github.com/cypress-io/github-action/issues/964
        if: ${{ (success() || failure()) }}
        id: reporter
        working-directory: 'e2e-tests'
        run: 'node cucumber-html-reporter.mjs'
        shell: bash

      - name: 'Upload end-to-end tests reports'
        id: cucumber-reports
        if: ${{ (success() || failure()) && (steps.reporter.outcome == 'success') }}
        uses: actions/upload-artifact@v3
        with:
          name: 'cucumber-reports-${{ steps.setup.outputs.browser }}'
          path: 'e2e-tests/cypress/reports/html'
          if-no-files-found: ignore

      - name: 'Upload end-to-end tests screenshots'
        id: cypress-screenshots
        if: ${{ (success() || failure()) && (steps.cypress.outcome == 'failure') }}
        uses: actions/upload-artifact@v3
        with:
          name: 'cypress-screenshots-${{ steps.setup.outputs.browser }}'
          path: 'e2e-tests/cypress/screenshots'
          if-no-files-found: ignore

I cannot see any use of continue-on-error in your example.

You are right, I did not include it because we are not using it and also for simplicity of the example.

That would be the only reason that conclusion and outcome would differ.

I don't have the same interpretation... conclusion and outcome always have a different meaning. The only difference comes when determining execution of the next step. With continue-on-error the conclusion value prevails, while without continue-on-error it prevails the outcome value.

It is probably not a good idea anyway to use continue-on-error because that could make the Cypress test look like it passed even if it failed.

True! That's why we are using if: ${{ (success() || failure()) }} .

You can find a good discussion about continue-on-error here: Properly show continue-on-error jobs/steps in PR UI

The example workflow you posted will not display the step outcomes after an error in the previous step in the job occurs. You could add if: always() to correct that.

      - name: 'Print previous step outcome/conclusion'
        if: always()

Right! Should have added the if: ${{ (success() || failure()) }} conditional... 😓

@MikeMcC399
Copy link
Collaborator

@ricardo-dematos

I observed the following when I tried this out (without using continue-on-error):

github-action test steps.cypress.conclusion steps.cypress.outcome
pass success success
fail failure failure

This agrees with the 'steps' context documentation, which describes how GitHub's actions/runner sets the values conclusion and outcome.

Your original post said "As long (as) cypress-io/github-action completes with success, it should report steps.cypress.conclusion=success." and this is exactly what I see.

In the screenshots you posted, the job e2e test was failing and there was no screenshot showing the value of steps.cypress.conclusion / steps.cypress.outcome. I would expect both of them to be set to failure in this case. I don't see an example of the action succeeding and conclusion being wrongly set to failure.

It should also be noted that the use of a custom command produces an error message:

A custom option can produce a failure. It can also be successful. We use it successfully with the Yarn Modern Plug'n'Play example command: yarn cypress run.

Again, if you think there is a bug, then a simple reproducible example together with logs would be extremely helpful.

@ricardo-dematos
Copy link
Author

To my understanding, the table, regardless of continue-on-error value, should be:

github-action test steps.cypress.conclusion steps.cypress.outcome
pass success success
fail success failure
error failure failure

The property used in the next step conditional, depending on continue-on-error, should be:

continue-on-error
true steps.cypress.conclusion The result of a completed step after continue-on-error is applied.
false steps.cypress.outcome The result of a completed step before continue-on-error is applied.

Notice that we are not using continue-on-error for the next step conditional.


In the screenshots you posted, the job e2e test was failing and there was no screenshot showing the value of steps.cypress.conclusion / steps.cypress.outcome. I would expect both of them to be set to failure in this case. I don't see an example of the action succeeding and conclusion being wrongly set to failure.

Screenshots with failing Cypress tests:

image

image

image

image

image

Screenshots without failing Cypress tests:

image

image

image

image

Screenshots with failing Cypress tests and continue-on-error: true on step 'Run end-to-end tests' :

image

image

image

image

Notice that all steps are marked as passed, although there are failing tests, hence this discussion about continue-on-error.


It should also be noted that the use of a custom command produces an error message:

A custom option can produce a failure. It can also be successful. We use it successfully with the Yarn Modern Plug'n'Play example command: yarn cypress run.

Shouldn't the action produce the step summary, even when the tests fail?


Again, if you think there is a bug, then a simple reproducible example together with logs would be extremely helpful.

The workflow, although not accompanied of the tests, is my example.

@MikeMcC399
Copy link
Collaborator

@ricardo-dematos

I tested using the regular way of using github-action without using the command option. I will retest using the command option like you are doing.

Shouldn't the action produce the step summary, even when the tests fail?

Using command disables many features of github-action including the test summary (see https://github.com/cypress-io/github-action#custom-test-command which refers to the publish-summary), so no, it is not expected for a test summary to be produced.

@MikeMcC399
Copy link
Collaborator

@ricardo-dematos

I'm sorry, but even after testing using the command option I still could not see a problem. I will have to pass this on to the Cypress experts now, as I have run out of ideas about how to help you.

@ricardo-dematos
Copy link
Author

ricardo-dematos commented Jul 17, 2023

Shouldn't the action produce the step summary, even when the tests fail?

Using command disables many features of github-action including the test summary (see https://github.com/cypress-io/github-action#custom-test-command which refers to the publish-summary), so no, it is not expected for a test summary to be produced.

I'm trying to use the cypress-io/github-action because I want the step summary and the annotations.

Using the command setting seems more and more, to me, the same has using the command line directly (run: ). 😔

I don't have the full picture and probably someone has a case for using the command setting, but I get the feeling that command and command-prefix should be removed from the cypress-io/github-action and users advised to use the command line directly, if they need something more specific. 🤔

@MikeMcC399
Copy link
Collaborator

@ricardo-dematos

  • Your comments address a much bigger issue than the step results. I have added a new enhancement request Release github-action for Node.js node20 #968 to cover some of this. I also mentioned about the disadvantages of the command option. I don't think there is any question of the command option being removed, since it is in use. Removing it would be a breaking change and would put those who use it at an unnecessary disadvantage. This of course does not mean that you need to continue using it.

@hckhanh
Copy link

hckhanh commented Jul 23, 2023

Shouldn't the action produce the step summary, even when the tests fail?

Using command disables many features of github-action including the test summary (see https://github.com/cypress-io/github-action#custom-test-command which refers to the publish-summary), so no, it is not expected for a test summary to be produced.

I'm trying to use the cypress-io/github-action because I want the step summary and the annotations.

Using the command setting seems more and more, to me, the same has using the command line directly (run: ). 😔

I don't have the full picture and probably someone has a case for using the command setting, but I get the feeling that command and command-prefix should be removed from the cypress-io/github-action and users advised to use the command line directly, if they need something more specific. 🤔

Is there anything else can work with Percy if command-prefix is removed?

@MikeMcC399
Copy link
Collaborator

@hckhanh

Is there anything else can work with Percy if command-prefix is removed?

command-prefix continues to be available as a parameter.

@MikeMcC399
Copy link
Collaborator

@ricardo-dematos

The release of cypress-io/github-action@v6 with full support of Node.js 20 should allow you to now avoid the use of the command parameter. You stated elsewhere that badeball/cypress-cucumber-preprocessor needs Node.js 18. (Note that I haven't tested if this works or not.)

@MikeMcC399
Copy link
Collaborator

@ricardo-dematos

Given the following configuration, when one of the tests fails both steps.cypress.conclusion and steps.cypress.outcome report failure , thus contradicting the documentation.

Your original problem description talks about status results which are set by the GitHub https://github.com/actions/runner software and documented in GitHub Actions > Learn GitHub Actions > Contexts > stepscontext which you quoted.

This may no longer be a problem for you, however if it is still an issue, then it is not something which cypress-io/github-action can resolve, so I suggest to close this issue now.

Resources

Here are other resource locations which might be helpful to you in case you need further assistance:

@MikeMcC399
Copy link
Collaborator

Closing, as previously suggested.

@MikeMcC399 MikeMcC399 closed this as not planned Won't fix, can't repro, duplicate, stale Nov 4, 2023
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

3 participants