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

TestLintFindProjectFromPath test fail on v1.6.25 #307

Closed
aminvakil opened this issue Jun 15, 2023 · 23 comments
Closed

TestLintFindProjectFromPath test fail on v1.6.25 #307

aminvakil opened this issue Jun 15, 2023 · 23 comments

Comments

@aminvakil
Copy link

Running go test -v ./ ./scripts/... on v1.6.25 fails with this error:

=== RUN   TestLintFindProjectFromPath
    linter_test.go:223: no error was detected though the project was found from path parameter
--- FAIL: TestLintFindProjectFromPath (0.00s)

Although it says no error was detected, but it fails, I haven't investigated more.
Full log: https://github.com/aminvakil/aur/actions/runs/5282061811/jobs/9556494234?pr=274#step:11:1968

Tests are running every night and it was fine during the previous upgrades, you can see more in github repository as well.

@rhysd
Copy link
Owner

rhysd commented Jun 16, 2023

All tests passed on my machine and on CI. I'll take a look tonight.

@aminvakil
Copy link
Author

In case it matters, here are versions used:

Package (5)           New Version  Net Change  Download Size

extra/perl-error      0.17029-4      0.04 MiB       0.02 MiB
extra/perl-mailtools  2.21-6         0.11 MiB       0.06 MiB
extra/perl-timedate   2.33-4         0.08 MiB       0.03 MiB
extra/git             2.41.0-1      38.57 MiB       6.75 MiB
extra/go              2:1.20.5-1   196.09 MiB      36.31 MiB

@rhysd
Copy link
Owner

rhysd commented Jun 16, 2023

The test case uses this file. It fails with the error message when this file does not exist.

Does the testdata directory exist in your test environment? The directory is necessary to run all tests.

@aminvakil
Copy link
Author

Yes.

I can build the package correctly on my system, but this fails on GHA in archlinux docker image with minimum packages installed, sometimes this happens when another dependency in building package should be added.

https://github.com/aminvakil/aur/actions/runs/5292060366/jobs/9578459512#step:11:177

CHANGELOG.md
CONTRIBUTING.md
Dockerfile
HomebrewFormula
LICENSE.txt
Makefile
README.md
action_metadata.go
action_metadata_test.go
all_webhooks.go
all_webhooks_test.go
ast.go
availability.go
availability_test.go
build
cmd
command.go
command_test.go
config.go
config_test.go
docs
error.go
error_test.go
expr.go
expr_ast.go
expr_insecure.go
expr_insecure_test.go
expr_lexer.go
expr_lexer_test.go
expr_parser.go
expr_parser_test.go
expr_sema.go
expr_sema_test.go
expr_test.go
expr_type.go
expr_type_test.go
fuzz
glob.go
glob_test.go
go.mod
go.sum
linter.go
linter_test.go
man
parse.go
pass.go
playground
popular_actions.go
popular_actions_test.go
process.go
process_test.go
project.go
quotes.go
quotes_test.go
reusable_workflow.go
reusable_workflow_test.go
rule.go
rule_action.go
rule_credentials.go
rule_deprecated_commands.go
rule_deprecated_commands_test.go
rule_env_var.go
rule_events.go
rule_expression.go
rule_glob.go
rule_id.go
rule_id_test.go
rule_job_needs.go
rule_matrix.go
rule_permissions.go
rule_pyflakes.go
rule_runner_label.go
rule_runner_label_test.go
rule_shell_name.go
rule_shellcheck.go
rule_shellcheck_test.go
rule_workflow_call.go
rule_workflow_call_test.go
scripts
testdata

https://github.com/aminvakil/aur/actions/runs/5292060366/jobs/9578459512#step:11:258

action_metadata
bench
config
err
examples
format
ok
projects
realworld
reusable_workflow_metadata

@aminvakil
Copy link
Author

aminvakil commented Jun 16, 2023

Also actionlint 1.6.24 built correctly last night on nightly CI.

https://github.com/aminvakil/aur/actions/runs/5285448927/jobs/9563946035

Edit: I just realized the test has been added in 1.6.25 .

@aminvakil
Copy link
Author

I also applied this 88c8bc0 and changed go test -v ./ ./scripts/... to go test -v ./..., still no luck.

@rhysd
Copy link
Owner

rhysd commented Jun 17, 2023

Thank you for the investigation. I'd like to try the same Arch Linux container. Would you tell me what container is used on your CI? Specifically

docker exec "$(cat ${container_id})" su devel sh -c "cd /pkg && makepkg -sri --check --noconfirm"

I want to know what $(cat ${container_id}) returns.

@aminvakil
Copy link
Author

This is a custom docker image maintained here: https://github.com/aminvakil/docker-archlinux/blob/master/Dockerfile_devel

It can get used from here: docker pull quay.io/aminvakil/archlinux:devel

But I'd suggest creating a PR in https://github.com/aminvakil/aur which everything has been taken care of, as docker image's entrypoint is systemd, it may not work on your system, but it's fine in GHA.

GHA will pick every PR which you make in aminvakil/aur, look for PKGBUILD changes, and runs workflow on that package, so you should just change actionlint/PKGBUILD, and it will get run and you can test.

Try adding a whitespace to PKGBUILD somewhere, I'll try to help if there was any issue with your PR.

@rhysd
Copy link
Owner

rhysd commented Jun 17, 2023

Thank you for the details. I think forking your repository and sending PR to the forked repository would be a good way for my investigation (since you need to approve PRs to run CI). I'll try it tomorrow.

@aminvakil
Copy link
Author

You won't need my approval.

This is the repository policy on running actions.
Only first-time contributors who recently created a GitHub account will require approval to run workflows.

@rhysd
Copy link
Owner

rhysd commented Jun 19, 2023

Today I could reproduce this issue in my forked repository. I'll investigate what is happening.

https://github.com/rhysd/aur-for-actionlint-bug/actions/runs/5312477943/jobs/9617034652

@aminvakil
Copy link
Author

Please tell me if there was any problem in running CI, ...

@rhysd
Copy link
Owner

rhysd commented Jun 20, 2023

Since .git directory didn't exist in tarball downloaded from GitHub, actionlint could not find the project directory (actionlint searches .git directory for detecting repository root directories).

I'll make a PR to fix test failure soon.

rhysd added a commit to rhysd/aur-for-actionlint-bug that referenced this issue Jun 20, 2023
@rhysd
Copy link
Owner

rhysd commented Jun 20, 2023

I made a PR. aminvakil/aur#281

After merging this PR, tests for v1.6.25 should pass.

@aminvakil
Copy link
Author

I made a PR. aminvakil/aur#281

After merging this PR, tests for v1.6.25 should pass.

Merged, shouldn't this get fixed here though? Why is .git presence necessary?

archlinux-github pushed a commit to archlinux/aur that referenced this issue Jun 20, 2023
@aminvakil
Copy link
Author

aminvakil/aur#274 tests passed and merged.

You can see the result https://aur.archlinux.org/cgit/aur.git/log/?h=actionlint

@rhysd
Copy link
Owner

rhysd commented Jun 20, 2023

Merged, shouldn't this get fixed here though? Why is .git presence necessary?

I'll add fix in this repository as well. However, for releasing v1.6.25, the fix in your side would be necessary until the next version (probably v1.6.26) is released.

.git is necessary because TestLintFindProjectFromPath checks if actionlint can find repository root directory. actionlint finds repository root by looking at .git directory. Currently actionlint repository's .git is used for the test. Git does not allow committing .git directory (git add testdata/.git is simply ignored) so it is tricky to prepare a fake .git directory for tests.

@aminvakil
Copy link
Author

Merged, shouldn't this get fixed here though? Why is .git presence necessary?

I'll add fix in this repository as well. However, for releasing v1.6.25, the fix in your side would be necessary until the next version (probably v1.6.26) is released.

Yes.

.git is necessary because TestLintFindProjectFromPath checks if actionlint can find repository root directory. actionlint finds repository root by looking at .git directory. Currently actionlint repository's .git is used for the test. Git does not allow committing .git directory (git add testdata/.git is simply ignored) so it is tricky to prepare a fake .git directory for tests.

I don't see the point of this test which does not differ aur/.git for example and mkdir -p .git and just looks for .git, as this is used for github actions, can't it at least look for .github folder presence?

I just checked and .github (which a file in it) can be added to a git commit.

However this is not related to this issue and it has been fixed, so I will close it.

Thanks for your help!

@rhysd
Copy link
Owner

rhysd commented Jun 20, 2023

can't it at least look for .github folder presence?

There is no guarantee that .github directory is put in root repository only. For example, .github directory may be put in somewhere else and symlinked to root of the repository. It's better to check both .git and .github IMO.

@rhysd
Copy link
Owner

rhysd commented Jun 20, 2023

Anyway, this is just an issue of test case. I'll add some hack such as dynamically creating a fake .git directory on running the test case under testdata directory. It would work.

rhysd added a commit that referenced this issue Jun 20, 2023
instead of using actionlint's `.git` directory
@rhysd
Copy link
Owner

rhysd commented Jun 20, 2023

I fixed the test case to work even if actionlint/.git doesn't exist. From v1.6.26 release, you can safely remove the change in aminvakil/aur#281.

@aminvakil
Copy link
Author

Thanks, I'll also create an actionlint-git package which uses latest commit of master, so it can be tested every night too.

@aminvakil
Copy link
Author

I've created https://aur.archlinux.org/packages/actionlint-git and tested here:
https://github.com/aminvakil/aur/actions/runs/5325589868/jobs/9646479531

It works without creating .git in check section, great!

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