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: better contributing documentation for act #765

Closed
ryuheechul opened this issue Aug 6, 2021 · 11 comments · Fixed by #766
Closed

Issue: better contributing documentation for act #765

ryuheechul opened this issue Aug 6, 2021 · 11 comments · Fixed by #766
Assignees
Labels
area/docs Documentation meta/resolved Issue has been fixed or valid solution has been provided (not workaround)

Comments

@ryuheechul
Copy link

System information

  • Operating System: macOS
  • Architecture: arm64
  • Apple M1: yes but running tests with go amd64 version with Rosetta
  • Docker version: 20.10.7
  • Docker image used in act: N/A
  • act version: main branch, 0ff204b

Expected behaviour

go test ./... run successfully

Actual behaviour

Many test files fail including
https://github.com/nektos/act/blob/0ff204b61523b0137d41c4479a132fde87211327/pkg/runner/step_context_test.go

Workflow and/or repository

N/A

Steps to reproduce

Either make test or go test ./...

act output

N/A

Description

It looks like many tests are broken? including

package runner
import (
"context"
"testing"
"github.com/nektos/act/pkg/common"
)
func TestStepContextExecutor(t *testing.T) {
platforms := map[string]string{
"ubuntu-latest": baseImage,
}
tables := []TestJobFileInfo{
{"testdata", "uses-and-run-in-one-step", "push", "Invalid run/uses syntax for job:test step:Test", platforms, ""},
{"testdata", "uses-github-empty", "push", "Expected format {org}/{repo}[/path]@ref", platforms, ""},
{"testdata", "uses-github-noref", "push", "Expected format {org}/{repo}[/path]@ref", platforms, ""},
{"testdata", "uses-github-root", "push", "", platforms, ""},
{"testdata", "uses-github-path", "push", "", platforms, ""},
{"testdata", "uses-docker-url", "push", "", platforms, ""},
{"testdata", "uses-github-full-sha", "push", "", platforms, ""},
{"testdata", "uses-github-short-sha", "push", "Unable to resolve action `actions/hello-world-docker-action@b136eb8`, the provided ref `b136eb8` is the shortened version of a commit SHA, which is not supported. Please use the full commit SHA `b136eb8894c5cb1dd5807da824be97ccdf9b5423` instead", platforms, ""},
}
// These tests are sufficient to only check syntax.
ctx := common.WithDryrun(context.Background(), true)
for _, table := range tables {
runTestJobFile(ctx, t, table)
}
}

❯ go test ./pkg/runner/step_context_test.go
# command-line-arguments [command-line-arguments.test]
pkg/runner/step_context_test.go:12:20: undefined: baseImage
pkg/runner/step_context_test.go:14:14: undefined: TestJobFileInfo
pkg/runner/step_context_test.go:27:3: undefined: runTestJobFile
FAIL    command-line-arguments [build failed]
FAIL

And indeed the file misses most of references.
just curious, if test files and testing is being ignored in the process of development?

Why raise this issue?

I tried to see if I could contribute to resolve this issue, #739
And I just cloned act for the first time and I find that not being able to run tests difficult to continue. Hence, became curious to see what's the stance of tests for this code/repository.

@ryuheechul ryuheechul added the kind/bug Something isn't working label Aug 6, 2021
@ChristopherHX
Copy link
Contributor

ChristopherHX commented Aug 6, 2021

They are working for me in my ide.

if test files and testing is being ignored in the process of development

All Tests are still required to succeed on linux and macOS, otherwise a PR will not be merged.

I'm using vscode + go tools, make shure you have go 1.16 and no older version.
I opened ./pkg/runner/step_context_test.go and pressed debug test ( a button over the test function added by the go tools for vscode).

go test ./pkg/runner/step_context_test.go fails with the same error on my windows machine, I guess this is not the right way to run a test.

  • You can also open the action tab of your fork
  • enable actions, you can keep "Close stale issues" and "release" disabled
  • select checks
  • press run workflow
  • select the branch with your changes
  • you will see your tests running on github

I hate the macOS Test suite for this project, becauses it fails far to often due to docker hub rate limits

@catthehacker
Copy link
Member

Not sure what you mean, go test ./... runs successfully and you provided example of running single test file which does not include required variable.

var baseImage string = "node:12-buster-slim"
func init() {
if p := os.Getenv("ACT_TEST_IMAGE"); p != "" {
baseImage = p
}
}

just curious, if test files and testing is being ignored in the process of development?

no, they are not

@catthehacker catthehacker added meta/invalid This doesn't seem right and removed kind/bug Something isn't working labels Aug 6, 2021
@catthehacker
Copy link
Member

  • Apple M1: yes but running tests with go amd64 version with Rosetta

You should be using arm64 version of Go

@ryuheechul
Copy link
Author

Thanks all for prompt responses!

I just figured out that go test ./pkg/runner/step_context_test.go is a wrong usage of testing in Go.
So I tried go test ./pkg/runner instead. Now no more the same issue (undefined ...) but failing with some other issues.

Since you all having no issues with running tests, something must have been wrong with my setup or I'm doing something wrong. I will try this later (maybe with an intel mac) and share my results.

Have a good weekend everyone :)

@catthehacker
Copy link
Member

catthehacker commented Aug 7, 2021

I just figured out that go test ./pkg/runner/step_context_test.go is a wrong usage of testing in Go.

well, it's not wrong usage in Go, just wrong in this project, depends if test depend on whole package or just file

@catthehacker
Copy link
Member

I guess the issue is solved so I'm going to close it. Ping me here or on Gitter if you still have issues.

I hate the macOS Test suite for this project, becauses it fails far to often due to docker hub rate limits

Please direct your hate towards Apple/Docker for having shitty licencing :) (because of that, docker can't be included in GitHub Actions runners and therefore special API keys for bypassing rate limits are not included like in ubuntu runners)

@ryuheechul
Copy link
Author

ryuheechul commented Aug 7, 2021

Thanks @catthehacker 👍🏼
I agree with closing the issue.
I can see that tests are running here fine, https://github.com/nektos/act/actions/runs/1094834668 (most recent tests)
(I should have found this one before opening the issue little too quickly!)

I understand macOS can be tricky to evaluate the build and test so I also tried with Github Codespace (Linux).
However, make test still failing on it and here are some pieces of error messeages from it.
(ran it multiple times for both master branch and most recent tag, 0.2.24)

--- FAIL: TestRunEventPullRequest (1.26s)
    runner_test.go:210: 
                Error Trace:    runner_test.go:210
                Error:          Expected nil, but got: Error response from daemon: Container 17f375729f595ddbc0aac432e7e1244911c54b277ebb90ff7e38926ee6b5ec03 is not running
                Test:           TestRunEventPullRequest
                Messages:       pull-request

...

*DRYRUN* [uses-github-root/test]   ❌  Failure - actions/hello-world-docker-action@b136eb8
time="2021-08-07T09:16:14Z" level=debug msg="b136eb8894c5cb1dd5807da824be97ccdf9b5423\ngithub.com/nektos/act/pkg/common.NewGitCloneExecutor.func1\n\t/workspaces/act/pkg/common/git.go:316\ngithub.com/nektos/act/pkg/runner.(*StepContext).Executor\n\t/workspaces/act/pkg/runner/step_context.go:110\ngithub.com/nektos/act/pkg/runner.(*RunContext).newStepExecutor.func1\n\t/workspaces/act/pkg/runner/run_context.go:265\ngithub.com/nektos/act/pkg/common.Executor.Then.func1\n\t/workspaces/act/pkg/common/executor.go:146\ngithub.com/nektos/act/pkg/common.Executor.Then.func1\n\t/workspaces/act/pkg/common/executor.go:133\ngithub.com/nektos/act/pkg/common.Executor.If.func1\n\t/workspaces/act/pkg/common/executor.go:154\ngithub.com/nektos/act/pkg/common.Executor.Finally.func1\n\t/workspaces/act/pkg/common/executor.go:180\ngithub.com/nektos/act/pkg/runner.(*runnerImpl).NewPlanExecutor.func1\n\t/workspaces/act/pkg/runner/runner.go:145\ngithub.com/nektos/act/pkg/common.Executor.ChannelError.func1\n\t/workspaces/act/pkg/common/executor.go:125\ngithub.com/nektos/act/pkg/common.NewParallelExecutor.func1.1\n\t/workspaces/act/pkg/common/executor.go:101\nruntime.goexit\n\t/usr/local/go/src/runtime/asm_amd64.s:1371\nshort SHA references are not supported\ngithub.com/nektos/act/pkg/common.NewGitCloneExecutor.func1\n\t/workspaces/act/pkg/common/git.go:316\ngithub.com/nektos/act/pkg/runner.(*StepContext).Executor\n\t/workspaces/act/pkg/runner/step_context.go:110\ngithub.com/nektos/act/pkg/runner.(*RunContext).newStepExecutor.func1\n\t/workspaces/act/pkg/runner/run_context.go:265\ngithub.com/nektos/act/pkg/common.Executor.Then.func1\n\t/workspaces/act/pkg/common/executor.go:146\ngithub.com/nektos/act/pkg/common.Executor.Then.func1\n\t/workspaces/act/pkg/common/executor.go:133\ngithub.com/nektos/act/pkg/common.Executor.If.func1\n\t/workspaces/act/pkg/common/executor.go:154\ngithub.com/nektos/act/pkg/common.Executor.Finally.func1\n\t/workspaces/act/pkg/common/executor.go:180\ngithub.com/nektos/act/pkg/runner.(*runnerImpl).NewPlanExecutor.func1\n\t/workspaces/act/pkg/runner/runner.go:145\ngithub.com/nektos/act/pkg/common.Executor.ChannelError.func1\n\t/workspaces/act/pkg/common/executor.go:125\ngithub.com/nektos/act/pkg/common.NewParallelExecutor.func1.1\n\t/workspaces/act/pkg/common/executor.go:101\nruntime.goexit\n\t/usr/local/go/src/runtime/asm_amd64.s:1371"
FAIL
FAIL    github.com/nektos/act/pkg/runner        60.106s
FAIL
make: *** [Makefile:31: test] Error 1

It's probably related to docker issue and I'm not intending to reopen this issue.
Simply just wanted to share what I saw just now.

@catthehacker
Copy link
Member

catthehacker commented Aug 7, 2021

I understand macOS can be tricky to evaluate the build and test so I also tried with Github Codespace (Linux).
However, make test still failing on it and here are some pieces of error messeages from it.
(ran it multiple times for both master branch and most recent tag, 0.2.24)

--- FAIL: TestRunEventPullRequest (1.26s)
    runner_test.go:210: 
                Error Trace:    runner_test.go:210
                Error:          Expected nil, but got: Error response from daemon: Container 17f375729f595ddbc0aac432e7e1244911c54b277ebb90ff7e38926ee6b5ec03 is not running
                Test:           TestRunEventPullRequest
                Messages:       pull-request

...

*DRYRUN* [uses-github-root/test]   ❌  Failure - actions/hello-world-docker-action@b136eb8
time="2021-08-07T09:16:14Z" level=debug msg="b136eb8894c5cb1dd5807da824be97ccdf9b5423\ngithub.com/nektos/act/pkg/common.NewGitCloneExecutor.func1\n\t/workspaces/act/pkg/common/git.go:316\ngithub.com/nektos/act/pkg/runner.(*StepContext).Executor\n\t/workspaces/act/pkg/runner/step_context.go:110\ngithub.com/nektos/act/pkg/runner.(*RunContext).newStepExecutor.func1\n\t/workspaces/act/pkg/runner/run_context.go:265\ngithub.com/nektos/act/pkg/common.Executor.Then.func1\n\t/workspaces/act/pkg/common/executor.go:146\ngithub.com/nektos/act/pkg/common.Executor.Then.func1\n\t/workspaces/act/pkg/common/executor.go:133\ngithub.com/nektos/act/pkg/common.Executor.If.func1\n\t/workspaces/act/pkg/common/executor.go:154\ngithub.com/nektos/act/pkg/common.Executor.Finally.func1\n\t/workspaces/act/pkg/common/executor.go:180\ngithub.com/nektos/act/pkg/runner.(*runnerImpl).NewPlanExecutor.func1\n\t/workspaces/act/pkg/runner/runner.go:145\ngithub.com/nektos/act/pkg/common.Executor.ChannelError.func1\n\t/workspaces/act/pkg/common/executor.go:125\ngithub.com/nektos/act/pkg/common.NewParallelExecutor.func1.1\n\t/workspaces/act/pkg/common/executor.go:101\nruntime.goexit\n\t/usr/local/go/src/runtime/asm_amd64.s:1371\nshort SHA references are not supported\ngithub.com/nektos/act/pkg/common.NewGitCloneExecutor.func1\n\t/workspaces/act/pkg/common/git.go:316\ngithub.com/nektos/act/pkg/runner.(*StepContext).Executor\n\t/workspaces/act/pkg/runner/step_context.go:110\ngithub.com/nektos/act/pkg/runner.(*RunContext).newStepExecutor.func1\n\t/workspaces/act/pkg/runner/run_context.go:265\ngithub.com/nektos/act/pkg/common.Executor.Then.func1\n\t/workspaces/act/pkg/common/executor.go:146\ngithub.com/nektos/act/pkg/common.Executor.Then.func1\n\t/workspaces/act/pkg/common/executor.go:133\ngithub.com/nektos/act/pkg/common.Executor.If.func1\n\t/workspaces/act/pkg/common/executor.go:154\ngithub.com/nektos/act/pkg/common.Executor.Finally.func1\n\t/workspaces/act/pkg/common/executor.go:180\ngithub.com/nektos/act/pkg/runner.(*runnerImpl).NewPlanExecutor.func1\n\t/workspaces/act/pkg/runner/runner.go:145\ngithub.com/nektos/act/pkg/common.Executor.ChannelError.func1\n\t/workspaces/act/pkg/common/executor.go:125\ngithub.com/nektos/act/pkg/common.NewParallelExecutor.func1.1\n\t/workspaces/act/pkg/common/executor.go:101\nruntime.goexit\n\t/usr/local/go/src/runtime/asm_amd64.s:1371"
FAIL
FAIL    github.com/nektos/act/pkg/runner        60.106s
FAIL
make: *** [Makefile:31: test] Error 1

It's probably related to docker issue and I'm not intending to reopen this issue.
Simply just wanted to share what I saw just now.

@ryuheechul
That's my fault on not documenting it and/or isolating but we test running different architecture container (tests run with linux/amd64 + one single test case runs linux/arm64 [if ran on amd64, if ran on arm64, everything will run as arm64])
You need to run docker run --privileged --rm tonistiigi/binfmt --install all or docker run --privileged --rm tonistiigi/binfmt --install amd64,arm64 and re-run tests

@ryuheechul
Copy link
Author

@catthehacker Cool and thanks!

So I ran it on Github Codespace and all tests have passed 👍🏼

$ docker run --privileged --rm tonistiigi/binfmt --install amd64,arm64
Unable to find image 'tonistiigi/binfmt:latest' locally
latest: Pulling from tonistiigi/binfmt
a5644ca467e1: Pull complete 
6f3748a79141: Pull complete 
Digest: sha256:ce4d5a2a6ac4a189047fca2d71cbd901cc7beebacf538be95fccb3aca87cb2ec
Status: Downloaded newer image for tonistiigi/binfmt:latest
installing: amd64 cannot write to /proc/sys/fs/binfmt_misc/register: write /proc/sys/fs/binfmt_misc/register: no such file or directory
installing: arm64 OK
{
  "supported": [
    "linux/amd64",
    "linux/arm64",
    "linux/386"
  ],
  "emulators": [
    "qemu-aarch64"
  ]
}
$ go test ./...
?       github.com/nektos/act   [no test files]
?       github.com/nektos/act/cmd       [no test files]
ok      github.com/nektos/act/pkg/common        (cached)
ok      github.com/nektos/act/pkg/container     (cached)
ok      github.com/nektos/act/pkg/model (cached)
ok      github.com/nektos/act/pkg/runner        111.241s

@ryuheechul
Copy link
Author

Anyone also didn't know what binfmt[_misc] was (just like me), you can read about it in the link below and it's quite fascinating.
https://medium.com/@tonistiigi/early-look-at-docker-containers-on-risc-v-40ed43b16b09

@catthehacker catthehacker reopened this Aug 7, 2021
@catthehacker catthehacker changed the title Issue: are tests broken? Issue: better contributing documentation for act Aug 7, 2021
@catthehacker
Copy link
Member

catthehacker commented Aug 7, 2021

I'm going to hijack this issue to improve docs.

@catthehacker catthehacker added stale-exempt Exempt from stale and removed meta/invalid This doesn't seem right labels Aug 7, 2021
@catthehacker catthehacker self-assigned this Aug 7, 2021
ryuheechul added a commit to ryuheechul/TIL that referenced this issue Aug 7, 2021
Another TIL thanks to this issue, nektos/act#765
ryuheechul added a commit to ryuheechul/TIL that referenced this issue Aug 7, 2021
Another TIL thanks to this issue, nektos/act#765
@cplee cplee closed this as completed in #766 Aug 9, 2021
@catthehacker catthehacker added area/docs Documentation meta/resolved Issue has been fixed or valid solution has been provided (not workaround) and removed stale-exempt Exempt from stale labels Aug 17, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/docs Documentation meta/resolved Issue has been fixed or valid solution has been provided (not workaround)
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants