search

google / starlark-go

Starlark in Go: the Starlark configuration language, implemented in Go
BSD 3-Clause "New" or "Revised" License
2.32k stars 212 forks source link

Add GitHub action for testing #384

Closed jayconrod closed 3 years ago

jayconrod commented 3 years ago

It runs all tests and confirms go.mod and go.sum are tidy, just like the Travis CI test.

The action runs with Go 1.16 and 1.17 on darwin, linux, and windows.

jayconrod commented 3 years ago

I'm not sure if there's a way to verify this works before merging it. I'm cargo-culting from https://github.com/google/wire, which did this migration yesterday.

adonovan commented 3 years ago

I'm not sure if there's a way to verify this works before merging it. I'm cargo-culting from https://github.com/google/wire, which did this migration yesterday.

I think the CI system runs the new configuration in PRs that edit the configuration. This is good for developers wanting to iterate, but I do wonder what are the security implications of allowing anyone who can send a PR to run arbitrary code. Actually, I know the answer: bitcoin mining, and the end of free Travis service.

Seems like it passed---though it took me a moment to realize that all the "The command foo exited with 0" were telling me "nothing to see here".

Thanks.

adonovan commented 3 years ago

Hmm, I thought the UI was telling me all was ok when I merged it, but the tests in fact fail. I'm not sure they fail spuriously though: IIRC, tests in one package reach into data files (assert.star) in another package, which works under Bazel and GOPATH, but not with Go modules. I have been meaning to fix it (but the cleanest fix wants go:embed, which is still young.)

--- FAIL: TestResolve (0.00s)
    chunkedfile.go:60: open /home/runner/go/src/go.starlark.net/resolve/testdata/resolve.star: no such file or directory
FAIL

Since the failures are not spurious, I don't see any need to roll back this change.