Skip to content

Add explicit support to GitHub Actions scripts for ".test/config.sh" …#47

Merged
tianon merged 1 commit into
docker-library:masterfrom
infosiftr:local-tests
Jun 24, 2022
Merged

Add explicit support to GitHub Actions scripts for ".test/config.sh" …#47
tianon merged 1 commit into
docker-library:masterfrom
infosiftr:local-tests

Conversation

@tianon

@tianon tianon commented Jun 14, 2022

Copy link
Copy Markdown
Member

…(and ".test/tests/xxx/")

Tested successfully via tianon/dockerfiles#223 😄

Closes #46

If .test/config.sh exists, it will be injected via --config on test/run.sh, which then also allows for additional tests in .test/tests/foo/run.sh.

@codecov-commenter

codecov-commenter commented Jun 14, 2022

Copy link
Copy Markdown

Codecov Report

Merging #47 (4632bf0) into master (b4880ac) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master      #47   +/-   ##
=======================================
  Coverage   76.83%   76.83%           
=======================================
  Files           5        5           
  Lines         587      587           
=======================================
  Hits          451      451           
  Misses        104      104           
  Partials       32       32           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b4880ac...4632bf0. Read the comment docs.

@tianon

tianon commented Jun 14, 2022

Copy link
Copy Markdown
Member Author

More complicated success: docker-library/docker#368

@tianon

tianon commented Jun 14, 2022

Copy link
Copy Markdown
Member Author

Comment thread scripts/github-actions/generate.sh Outdated

@williamdes williamdes left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks perfect, thank you !

"set -- " + (.tags[0] | @sh),
# https://github.com/docker-library/bashbrew/issues/46#issuecomment-1152567694 (allow local test config / tests)
"if [ -s ./.test/config.sh ]; then set -- --config ~/oi/test/config.sh --config ./.test/config.sh \"$@\"; fi",
"~/oi/test/run.sh \"$@\""

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you echo to gha output that you used the override config
So people do not have to debug the script

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you look at https://github.com/tianon/dockerfiles/runs/6889534295?check_suite_focus=true#step:7:10, we run all these with set -x so we kind of get that "for free" 😅

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah nice, explicit github ci output would be nice but okay for me 😆

@tianon
tianon requested a review from yosifkit June 24, 2022 18:10
@tianon
tianon merged commit b4cc436 into docker-library:master Jun 24, 2022
@tianon
tianon deleted the local-tests branch June 24, 2022 18:10
@williamdes

Copy link
Copy Markdown

I tried this out and had this error in my output: https://github.com/williamdes/docker/runs/7055760237?check_suite_focus=true#step:7:14

/home/runner/oi/test/run.sh: line 243: testPaths[$t]: unbound variable

@yosifkit

Copy link
Copy Markdown
Member

I think you need to move the tests directory from the root of your repo into the .test directory (next to the config.sh)

.test/
   config.sh
   tests/
      phpmyadmin-import/

@williamdes

Copy link
Copy Markdown

Thanks to the new action is is simpler to make this work: phpmyadmin/docker#389

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

Successfully merging this pull request may close these issues.

Allow to override the repository name and config location

4 participants