Add the ability to pass arbitrary keyword arguments to the Pkg.test function

[DilumAluthge]

This PR adds the ability to pass arbitrary keyword arguments to the Pkg.test function.

Example usage:

steps:
  - uses: julia-actions/julia-runtest@v1
    with:
      additional_keyword_arguments: 'foo = true, bar = "hello", baz = world'

Motivation

There may be keyword arguments to Pkg.test that we don't want to explicitly hardcode into the julia-runtest action. For example, on Julia nightly, there is currently a allow_earlier_backwards_compatible_versions keyword argument that you can pass to Pkg.test (see https://github.com/JuliaLang/Pkg.jl/blob/f0731405b557b8c04788f7fc6ca03d5a2e673280/src/Pkg.jl#L181-L229 and JuliaLang/Pkg.jl#2439 for details).

This keyword argument is experimental, and may be removed in future minor releases of Julia. Therefore, we don't want to explicitly hardcode it as an input to the julia-runtest action.

This PR allows the user to provide arbitrary key-value pairs that are passed as keyword arguments to Pkg.test without needing to create new inputs for those keyword arguments.

[codecov-io]

Codecov Report

Merging #39 (5f54fbb) into master (eda4346) will not change coverage.
The diff coverage is n/a.

@@            Coverage Diff            @@
##            master       #39   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files            1         1           
  Lines            2         2           
=========================================
  Hits             2         2           

---

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 eda4346...5f54fbb. Read the comment docs.

[DilumAluthge]

I don't think this is the best approach, it's far too complicated.

[DilumAluthge]

@SaschaMann If you are looking to implement #47, I think this PR was the right approach.

In this PR, I was mainly unhappy about the use of Meta.parse, but it might be unavoidable.

[DilumAluthge]

@SaschaMann Do you want to take over this branch?

[SaschaMann]

I'll create a new one.

My naive approach would have been to do something like

    - run: |
        # The Julia command that will be executed
        julia_cmd=( julia ${{ input.julia-args }} --color=yes --depwarn=${{ inputs.depwarn }} --inline=${{ inputs.inline }} --project=${{ inputs.project }} -e 'import Pkg;include(joinpath(ENV["GITHUB_ACTION_PATH"], "kwargs.jl"));kwargs = Kwargs.kwargs(;coverage = :(${{ inputs.coverage }}),force_latest_compatible_version = :(${{ inputs.force_latest_compatible_version }}), julia_args = ["${{ input.julia-test-args }} --check-bounds=${{ inputs.check_bounds }}"]);Pkg.test(; kwargs...)' )

Could you elaborate your reasoning behind the custom syntax for the input and your approach?

[DilumAluthge]

Could you elaborate your reasoning behind the custom syntax for the input and your approach?

Yeah. The goal of this PR was that we wouldn't lock ourselves into just supporting coverage, julia_args, and test_args, but instead we would be able to support any future additions to the Pkg.test kwarg list without needing to make any modifications to this action. AFAICT, with your version, if in the future a new kwarg is added to Pkg.test, we then need to add a new input to this action, which means that the list of inputs for this action will keep getting longer. In contrast, the goal of my PR here is that once this PR was implemented, we would not need to add more inputs to this action in the future. But as you can see, the implementation of this PR gets really ugly with all of the Meta.parses.

[SaschaMann]

Yeah. The goal of this PR was that we wouldn't lock ourselves into just supporting coverage, julia_args, and test_args, but instead we would be able to support any future additions to the Pkg.test kwarg list without needing to make any modifications to this action. AFAICT, with your version, if in the future a new kwarg is added to Pkg.test, we then need to add a new input to this action, which means that the list of inputs for this action will keep getting longer.

That's true. Do you happen to know if there are any plans to add more kwargs? To me that risk seems more managable than the complexity of this approach but I also don't know how often args to Pkg.test are added.

[DilumAluthge]

It occurs to me that there's actually two issues:

1. New public kwargs
2. New non-public kwargs

I'm hoping that we won't add too many more public kwargs to Pkg.test. But we do still do it from time to time. For example, in 1.9, allow_reresolve will become a public kwarg.

For new public kwargs, we can always add more inputs to action.yml. It will get kind of unwieldy over time, but at least we can do it.

For new non-public kwargs, I don't think we have any ability to support them at all (if we don't do the approach in this PR). Because we certainly can't make it part of the public API of this action if it's not part of the public API of Pkg.

allow_reresolve was actually introduced as a non-public kwarg in 1.7. So at that time, users would have had no way of using that kwarg in this action. AFAICT, the only way for this action to support non-public kwargs is by this PR.

But this PR is so complicated (or at least it looks complicated), and I don't know if the cost of the complexity is outweighed by the benefit of supporting non-public kwargs.

[SaschaMann]

Because we certainly can't make it part of the public API of this action if it's not part of the public API of Pkg.

I think if we document its status as non-public clearly, we can get away with it. We may end up with a bunch of deprecated/version-specific inputs in the long run but that may just be the price we have to pay.

---

But this PR is so complicated (or at least it looks complicated), and I don't know if the cost of the complexity is outweighed by the benefit of supporting non-public kwargs.

I will have a think if and how we could simplify the approach but it's gonna take some time.