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

Issue: if ${{ env.ACT }} not working on master #940

Closed
jayvdb opened this issue Dec 23, 2021 · 5 comments · Fixed by #965
Closed

Issue: if ${{ env.ACT }} not working on master #940

jayvdb opened this issue Dec 23, 2021 · 5 comments · Fixed by #965
Labels
kind/bug Something isn't working

Comments

@jayvdb
Copy link

jayvdb commented Dec 23, 2021

System information

  • Operating System: macOS
  • Architecture: x64
  • Apple M1: no
  • Docker version: 20.10.10
  • Docker image used in act:
  • act version: ed01f46 (also a few few commits earlier 9868e13 )

Expected behaviour

Assuming https://github.com/nektos/act#skipping-steps is still current for master

Steps with if: ${{ !env.ACT }} should be skipped.

Steps with if: ${{ env.ACT }} should not be skipped.

Actual behaviour

Steps with if: ${{ env.ACT }} are skipped.

Steps with if: ${{ !env.ACT }} are not skipped.

Workflow and/or repository

name: Foo
on:
  workflow_dispatch:
jobs:
  bar:
    name: Baz
    runs-on: ubuntu-latest
    steps:
      - name: Checkout action
        uses: actions/checkout@v2
      - name: Show env.ACT
        shell: bash
        run: echo "$ACT"
      - id: setup-python
        if: ${{ env.ACT }}
        name: Setup Python
        uses: actions/setup-python@v2
        with:
          python-version: '3.7'
      - if: ${{ !env.ACT }}
        name: not-ACT
        shell: bash
        run: echo "$ACT"

Steps to reproduce

  1. Install act from master
  2. Run on workflows that were previous working correctly with if: ${{ env.ACT }} or if: ${{ !env.ACT }} on v0.2.25
  3. Add "echo $ACT" to show it is being set to true

act output

[Foo/Baz] 🚀  Start image=catthehacker/ubuntu:act-latest
...
[Foo/Baz]   🐳  docker cp src=/Users/jayvdb/.../. dst=/Users/jayvdb/...
[Foo/Baz]   🐳  docker exec cmd=[mkdir -p /Users/jayvdb/...] user= workdir=
[Foo/Baz] ⭐  Run Checkout action
[Foo/Baz]   ✅  Success - Checkout action
[Foo/Baz] ⭐  Run Show env.ACT
[Foo/Baz]   🐳  docker exec cmd=[bash --noprofile --norc -e -o pipefail /var/run/act/workflow/1.sh] user= workdir=
| true
[Foo/Baz]   ✅  Success - Show env.ACT
[Foo/Baz] ⭐  Run not-ACT
[Foo/Baz]   🐳  docker exec cmd=[bash --noprofile --norc -e -o pipefail /var/run/act/workflow/setup-python.sh] user= workdir=
| true
[Foo/Baz]   ✅  Success - not-ACT
@jayvdb jayvdb added the kind/bug Something isn't working label Dec 23, 2021
@ChristopherHX
Copy link
Contributor

I can confirm, this is broken in current master since 1891c72.

However #908 will fix this issue again, together with an improved expression evaluator.
I wonder why this feature doesn't have a testcase.

@BlackDex
Copy link
Contributor

Maybe @ZauberNerd can add a proper test for this?
Something like:

name: if-env-act-test
on: push

jobs:
  if_env_test:
    name: Test if env.ACT matching
    runs-on: ubuntu-latest
    steps:
      # Should RUN, since we are running in act
      - name: Positive env.ACT match
        if: ${{ env.ACT }}
        shell: bash
        run: |
          echo "This workflow is run using act, continue!"
          echo "ACT: $ACT"
          exit 0

      # Should SKIP, since we are running in act
      - name: Negative env.ACT match
        if: ${{ !env.ACT }}
        shell: bash
        run: |
          echo "This should be skipped since this workflow is run using act, fail!"
          echo "ACT: $ACT"
          exit 1

@ZauberNerd
Copy link
Contributor

@BlackDex I don't see how that test relates to the expression evaluator. IMHO it should be a separate PR to be merged after #908 has been merged.

@BlackDex
Copy link
Contributor

BlackDex commented Jan 20, 2022

@ZauberNerd Well, the expression evaluator changes from PR #840 broke this and according to @ChristopherHX PR #908 fixes that issue, so maybe wise to have a test in there which prevents any further breaking of this when working on this feature?

It sounds logical to me to have a test added to a PR which fixes a specific issue doesn't it?

@ChristopherHX
Copy link
Contributor

@BlackDex I added your testworkflow in a new PullRequest, since the bug was fixed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants