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

Add test_arg input and populate to test_args in test() #73

Merged
merged 3 commits into from
Jul 23, 2024

Conversation

sschlenkrich
Copy link
Contributor

This PR aims at increasing flexibility for running Julia tests. The string value of the new test_arg input is passed to test and can be accessed in Julia via the ARGS variable.

Use cases are, for example, distinuishing runs of fast and slow tests.

@sschlenkrich sschlenkrich requested a review from a team as a code owner February 27, 2023 09:07
@codecov
Copy link

codecov bot commented Feb 27, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 100.00%. Comparing base (d0c4f09) to head (ed29cc7).

Additional details and impacted files
@@            Coverage Diff            @@
##              main       #73   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files            1         1           
  Lines            2         2           
=========================================
  Hits             2         2           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@sschlenkrich sschlenkrich marked this pull request as draft May 31, 2023 16:23
@sschlenkrich sschlenkrich marked this pull request as ready for review May 31, 2023 16:33
@adamslc
Copy link

adamslc commented Jun 29, 2023

Is there anything blocking the PR? I would also find the functionality helpful.

I see the in #39 there was a concern about adding too many additional keyword args, but this seems like a very basic piece of functionality.

@willow-ahrens
Copy link

I would also find this functionality useful, thanks for filing the PR!

@sschlenkrich
Copy link
Contributor Author

sschlenkrich commented Jun 7, 2024

I rebased my changes on the current main branch. Hopefully, this gives another impuls for a review of this PR.

@mhauru
Copy link

mhauru commented Jul 19, 2024

I would very much like to see this merged, test_args is a standard part of how Pkg.test operates, and would be great to have the Action support it.

Could we get this to support multiple test_ags, since Pkg.test supports that too? You could leverage Julia's own arg parsing by, in the action.yml, passing inputs as julia test.harness.jl {{ inputs.test_args }}, and then calling Kwargs.kwargs with test_args=ARGS. I think using ARGS would be very natural here, because what Pkg.test does is it makes the test_args available to runtests.jl as ARGS. I'm happy to make a PR into this PR too if that's helpful.

@sschlenkrich
Copy link
Contributor Author

Thanks @mhauru for your comment. Yes, I am totally fine if you add/amend to my PR.

To be honest, I am a little bit reluctant to put to much effort in this PR by myself. This PR is open for quite a while. But the maintainers did not find the time for a review.

@IanButterworth
Copy link
Member

Sorry your PR was opened in a period of low maintenance and it fell down in attention. Please feel free to bump when the PR is considered ready again for review/merge.

@sschlenkrich
Copy link
Contributor Author

Thanks @IanButterworth for your comment! I rebased my proposed changes on the latest main branch. From my perspective this PR is ready for review. Could you have a look?

I would suggest to first integrate this PR and then @mhauru could add support for multiple arguments.

Copy link
Member

@IanButterworth IanButterworth left a comment

Choose a reason for hiding this comment

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

I think this could do with an example in the readme

@sschlenkrich
Copy link
Contributor Author

Documentation and example is a fair point. I just added a paragraph to the readme. @IanButterworth, could you have a look if this works for you?

action.yml Outdated Show resolved Hide resolved
@IanButterworth IanButterworth merged commit e164761 into julia-actions:main Jul 23, 2024
32 checks passed
@IanButterworth
Copy link
Member

@mhauru please go ahead and PR the multiple args work and we can make a release after that

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.

6 participants