Skip to content

Conversation

@DilumAluthge
Copy link
Member

@DilumAluthge DilumAluthge commented Jun 13, 2019

Fixes #518
Fixes #981
Fixes #1241


In a source build of Julia, you can run specific test sets as such:

./julia test/runtests.jl bitarray math

In a binary install, you can accomplish the same thing with:

Base.runtests(["bitarray", "math"])

Base.runtests(tests) accomplishes this (see here) by passing the contents of the first positional argument test as the command line arguments to test/runtests.jl.

However, as far as I can tell, there is currently no functionality built into Pkg that allows you to pass any arguments to a package's test/runtests.jl file when running the package tests.

This pull request adds two features:

  • Pass flags to the julia process that runs a package's tests. The flags are specified in the julia_flags keyword argument to Pkg.test.
  • Pass positional command-line arguments to test/runtests.jl. The arguments are specified in the test_args keyword argument to Pkg.test and are available inside test/runtests.jl as ARGS[1], ARGS[2], etc.

Example usage:

# equivalent to running `julia test/runtests.jl`
Pkg.test("HelloWorld")
# equivalent to running `julia test/runtests.jl foo bar baz`
Pkg.test("HelloWorld"; test_args = ["foo", "bar", "baz"])
# equivalent to running `julia --math-mode=ieee test/runtests.jl`
Pkg.test("HelloWorld"; julia_flags = ["--math-mode=ieee"])
# equivalent to running `julia --math-mode=ieee test/runtests.jl foo bar baz`
Pkg.test("HelloWorld"; test_args = ["foo", "bar", "baz"], julia_flags = ["--math-mode=ieee"])

This pull request only adds the functionality for passing the command line arguments to test/runtests.jl. It is up to the author of a package to add the logic for processing the command line arguments inside their test/runtests.jl file. However, this should be relatively straightforward. For example, if this is the current test/runtests.jl file:

using HelloWorld
using Test

@testset "HelloWorld.jl" begin
    @testset "foo" begin
        @test HelloWorld.foo(1) == 2
    end
    @testset "bar" begin
        @test HelloWorld.bar(2) == 6
    end
    @testset "baz" begin
        @test HelloWorld.baz(3) == 12
    end
end

Then you could modify it to look like this:

using HelloWorld
using Test

@testset "HelloWorld.jl" begin
    if isempty(ARGS) || "all" in ARGS
        all_tests = true
    else
        all_tests = false
    end
    if all_tests || "foo" in ARGS
        @testset "foo" begin
            @test HelloWorld.foo(1) == 2
        end
    end
    if all_tests || "bar" in ARGS
        @testset "bar" begin
            @test HelloWorld.bar(2) == 6
        end
    end
    if all_tests || "baz" in ARGS
        @testset "baz" begin
            @test HelloWorld.baz(3) == 12
        end
    end
end

And now if you run Pkg.test("HelloWorld"; test_args = ["foo", "baz"]), then only the foo and baz testsets will be run.

@KristofferC
Copy link
Member

KristofferC commented Jun 17, 2019

I like it. We'll have to discuss a bit on the exact API for the command line args but this feels pretty ok.

bors bot added a commit that referenced this pull request Jun 17, 2019
@KristofferC
Copy link
Member

bors try

@bors
Copy link
Contributor

bors bot commented Jun 17, 2019

try

Already running a review

@bors
Copy link
Contributor

bors bot commented Jun 17, 2019

@KristofferC
Copy link
Member

KristofferC commented Jun 17, 2019

We probably also want to be able to give julia startup command line options. For example --track-allocations. So perhaps there should be julia_args, test_args or something?

@DilumAluthge
Copy link
Member Author

DilumAluthge commented Jun 17, 2019

We probably also want to be able to give julia startup command line options. For example --track-allocations. So perhaps there should be julia_args, test_args or something?

I like julia_args and test_args

Actually I wonder if it will be a little confusing to use "args" twice. How about julia_flags and test_args?

We're going with julia_args and test_args.

@DilumAluthge
Copy link
Member Author

DilumAluthge commented Jun 17, 2019

I renamed args to test_args, and I added a new keyword argument julia_flags.

We're going with julia_args and test_args.

@fredrikekre
Copy link
Member

Is it not better with julia_args and test_args?
We should also think of a Pkg REPL syntax for this. I am thinking only test_args needs to be supported, since julia_args should not be so commonly used. Maybe something like

pkg> test Example -- arg1 arg2

or something, i.e. using -- as a separator. This could be implemented in a separate PR of course.

@DilumAluthge
Copy link
Member Author

DilumAluthge commented Jun 19, 2019

We're going with julia_args and test_args.

@DilumAluthge
Copy link
Member Author

DilumAluthge commented Jun 21, 2019

We should also think of a Pkg REPL syntax for this. I am thinking only test_args needs to be supported, since julia_args should not be so commonly used.

Could we support both? Maybe something like:

pkg> test Example --julia-args --track-allocation=all --test-args arg1 arg2

But yeah I definitely agree that we should implement this in a separate pull request.

@StefanKarpinski
Copy link
Member

That looks quite a bit worse to me. At that point, shouldn't you just be calling the test script manually?

@fredrikekre
Copy link
Member

I agree, we have not even had a feature request for customizing julia flags

@DilumAluthge
Copy link
Member Author

That looks quite a bit worse to me. At that point, shouldn't you just be calling the test script manually?

Fair enough. So if we add REPL support, it will only be for test args.

@DilumAluthge
Copy link
Member Author

I've made the requested changes!

Also, in the interest of "eating our own dog food", I've updated the Pkg test suite to support running specific testsets. I've added an example to .travis.yml to demonstrate.

@cmcaine
Copy link

cmcaine commented Jul 2, 2019

That looks quite a bit worse to me. At that point, shouldn't you just be calling the test script manually?

If you want to run a test script manually with julia I think you have to Pkg.add() all of the deps and extras for the package before it will work, so it could be a moderate pain to do.

Kind of off-topic:

Could solve that in another PR by exposing (if they are not already exposed) some functions for getting the right LOAD_PATH from a Project.toml (like activate, but including extras, and possibly not actually changing LOAD_PATH for you, just returning what you should use).

An extra use case for this is running ancillary test scripts (e.g. test/compare_examples.jl in Gadfly (it diffs binary outputs that have been saved to disk from master and the feature branch for regression tests)).

@fredrikekre
Copy link
Member

Kind of off-topic:
Could solve that in another PR by exposing (if they are not already exposed) some functions for getting the right LOAD_PATH from a Project.toml (like activate, but including extras, and possibly not actually changing LOAD_PATH for you, just returning what you should use).

This is exposed(ish) in Julia 1.2; you can there run tests as

julia --project=test test/runtests.jl

@cmcaine
Copy link

cmcaine commented Jul 2, 2019

As mentioned on slack, I think that would require copying the Project.toml to the test directory and making some manual changes first.

@DilumAluthge
Copy link
Member Author

I think what @fredrikekre is referring to is that in Julia 1.2 and higher, you specify the test dependencies in test/Project.toml, not in the main Project.toml file.

@fredrikekre
Copy link
Member

Looks pretty good now, but

Also, in the interest of "eating our own dog food", I've updated the Pkg test suite to support running specific testsets. I've added an example to .travis.yml to demonstrate.

can we undo that? I don't think that is necessary and just results in a huge diff.

@DilumAluthge
Copy link
Member Author

Also, in the interest of "eating our own dog food", I've updated the Pkg test suite to support running specific testsets. I've added an example to .travis.yml to demonstrate.

can we undo that? I don't think that is necessary and just results in a huge diff.

Alright I think I've reverted all of those changes!

@fredrikekre
Copy link
Member

bors try

bors bot added a commit that referenced this pull request Jul 26, 2019
Co-authored-by: Dilum Aluthge <dilum@aluthge.com>
Co-authored-by: Fredrik Ekre <ekrefredrik@gmail.com>
@fredrikekre
Copy link
Member

I simplified the tests, and fixed running the old code-path.

bors try

@bors
Copy link
Contributor

bors bot commented Jul 26, 2019

try

Already running a review

@bors
Copy link
Contributor

bors bot commented Jul 26, 2019

try

Build failed

@fredrikekre
Copy link
Member

bors r+

bors bot added a commit that referenced this pull request Jul 26, 2019
1226: [RFC] Allow Pkg.test to pass command-line arguments to test/runtests.jl r=fredrikekre a=DilumAluthge

Fixes #518 
Fixes #981 
Fixes #1241   



Co-authored-by: Dilum Aluthge <dilum@aluthge.com>
@bors
Copy link
Contributor

bors bot commented Jul 26, 2019

@rfourquet
Copy link
Member

This is really neat, thanks! Also, my +1 to Pkg REPL support for test_args.

bors bot added a commit to CliMA/ClimateMachine.jl that referenced this pull request Mar 6, 2020
813: improve formatting of simtime and endtime in logging r=simonbyrne a=simonbyrne

# Description

Switches to using a fixed format for printing simtime and endtime, and print denominator in callback.

Old:
```
┌ Info: Update
│ simtime = 2.6159163716067161e+02
│ runtime = 00:01:59
└ norm(Q) = 3.0116687421160989e+09
```

New:
```
┌ Info: Update
│ simtime =    15.34 /  1000.00
│ runtime = 00:28:31
└ norm(Q) = 3.0115880375677762e+09
```
<!--- Please leave the following section --->

# For review by CLIMA developers

- [x] There are no open pull requests for this already
- [x] CLIMA developers with relevant expertise have been assigned to review this submission
- [ ] The code conforms to the [style guidelines](https://climate-machine.github.io/CLIMA/latest/CodingConventions.html) and has consistent naming conventions
- [ ] This code does what it is technically intended to do (all numerics make sense physically and/or computationally)


815: Make tests flexibly callable r=charleskawczynski a=charleskawczynski

<!--
Thanks for submitting code to CLIMA, the Climate Machine.

Before continuing, please be sure you have:

1. Written and run all necessary tests with CLIMA by including `tests/runtests.jl`
2. Followed all necessary [style guidelines](https://climate-machine.github.io/CLIMA/latest/CodingConventions.html)
3. Identified key contributors to review this submission
-->

# Description

This adds flexibility to call specific tests, using [command line args](JuliaLang/Pkg.jl#1226).

Now, we can call a specific test as follows

```
julia --project test/runtests.jl Common
Starting tests for Common
Starting tests for MoistThermodynamics
[ Info: CUDAdrv.jl failed to initialize, GPU functionality unavailable (set JULIA_CUDA_SILENT or JULIA_CUDA_VERBOSE to silence or expand this message)
Completed tests for MoistThermodynamics, 30 seconds elapsed
Starting tests for PlanetParameters
Completed tests for PlanetParameters, 0 seconds elapsed
Completed tests for Common, 31 seconds elapsed
Test Summary: | Pass  Total
CLIMA         |  457    457
```

# For review by CLIMA developers

- [x] There are no open pull requests for this already
- [x] CLIMA developers with relevant expertise have been assigned to review this submission
- [ ] The code conforms to the [style guidelines](https://climate-machine.github.io/CLIMA/latest/CodingConventions.html) and has consistent naming conventions
- [ ] This code does what it is technically intended to do (all numerics make sense physically and/or computationally)


Co-authored-by: Simon Byrne <simonbyrne@gmail.com>
Co-authored-by: Charles Kawczynski <kawczynski.charles@gmail.com>
bors bot added a commit to CliMA/ClimateMachine.jl that referenced this pull request Mar 7, 2020
802: Add `ParameterSets` r=charleskawczynski a=charleskawczynski

<!--
Thanks for submitting code to CLIMA, the Climate Machine.

Before continuing, please be sure you have:

1. Written and run all necessary tests with CLIMA by including `tests/runtests.jl`
2. Followed all necessary [style guidelines](https://climate-machine.github.io/CLIMA/latest/CodingConventions.html)
3. Identified key contributors to review this submission
-->

# Description

Adds `ParameterSets`, for flexibly handling
 - Planet constants
 - Free parameters

This should allow us to compute planet constants, or free parameters, in a flexible way. Specifically, we aim to be able to overload methods such as
 - `grav(::AbstractParameterSet)` (a planet constant) or
 - `C_smag(::AbstractParameterSet)` (a free parameter)

in the driver.

<!--- Please leave the following section --->

# For review by CLIMA developers

- [x] There are no open pull requests for this already
- [x] CLIMA developers with relevant expertise have been assigned to review this submission
- [ ] The code conforms to the [style guidelines](https://climate-machine.github.io/CLIMA/latest/CodingConventions.html) and has consistent naming conventions
- [ ] This code does what it is technically intended to do (all numerics make sense physically and/or computationally)


811: Replaces `forcecpu` with `init_on_cpu` for clarity r=akshaysridhar a=akshaysridhar

<!--
Thanks for submitting code to CLIMA, the Climate Machine.

Before continuing, please be sure you have:

1. Written and run all necessary tests with CLIMA by including `tests/runtests.jl`
2. Followed all necessary [style guidelines](https://climate-machine.github.io/CLIMA/latest/CodingConventions.html)
3. Identified key contributors to review this submission
-->

# Description

Replaces instances of `forcecpu` keyword with `init_on_cpu` to clarify its functionality. It only enables initialisation on CPU, and does not force the run to be on CPU. (use cases: Currently CPU initialisation needs to be enabled when using random number generators for the initial condition)

<!--- Please leave the following section --->

# For review by CLIMA developers

- [x] There are no open pull requests for this already
- [x] CLIMA developers with relevant expertise have been assigned to review this submission
- [ ] The code conforms to the [style guidelines](https://climate-machine.github.io/CLIMA/latest/CodingConventions.html) and has consistent naming conventions
- [x] This code does what it is technically intended to do (all numerics make sense physically and/or computationally)


815: Make tests flexibly callable r=charleskawczynski a=charleskawczynski

<!--
Thanks for submitting code to CLIMA, the Climate Machine.

Before continuing, please be sure you have:

1. Written and run all necessary tests with CLIMA by including `tests/runtests.jl`
2. Followed all necessary [style guidelines](https://climate-machine.github.io/CLIMA/latest/CodingConventions.html)
3. Identified key contributors to review this submission
-->

# Description

This adds flexibility to call specific tests, using [command line args](JuliaLang/Pkg.jl#1226).

Now, we can call a specific test as follows

```
julia --project test/runtests.jl Common
Starting tests for Common
Starting tests for MoistThermodynamics
[ Info: CUDAdrv.jl failed to initialize, GPU functionality unavailable (set JULIA_CUDA_SILENT or JULIA_CUDA_VERBOSE to silence or expand this message)
Completed tests for MoistThermodynamics, 30 seconds elapsed
Starting tests for PlanetParameters
Completed tests for PlanetParameters, 0 seconds elapsed
Completed tests for Common, 31 seconds elapsed
Test Summary: | Pass  Total
CLIMA         |  457    457
```

# For review by CLIMA developers

- [x] There are no open pull requests for this already
- [x] CLIMA developers with relevant expertise have been assigned to review this submission
- [ ] The code conforms to the [style guidelines](https://climate-machine.github.io/CLIMA/latest/CodingConventions.html) and has consistent naming conventions
- [ ] This code does what it is technically intended to do (all numerics make sense physically and/or computationally)


Co-authored-by: Charles Kawczynski <kawczynski.charles@gmail.com>
Co-authored-by: asraero <asridhar@caltech.edu>
bors bot added a commit to CliMA/ClimateMachine.jl that referenced this pull request Mar 7, 2020
815: Make tests flexibly callable r=charleskawczynski a=charleskawczynski

<!--
Thanks for submitting code to CLIMA, the Climate Machine.

Before continuing, please be sure you have:

1. Written and run all necessary tests with CLIMA by including `tests/runtests.jl`
2. Followed all necessary [style guidelines](https://climate-machine.github.io/CLIMA/latest/CodingConventions.html)
3. Identified key contributors to review this submission
-->

# Description

This adds flexibility to call specific tests, using [command line args](JuliaLang/Pkg.jl#1226).

Now, we can call a specific test as follows

```
julia --project test/runtests.jl Common
Starting tests for Common
Starting tests for MoistThermodynamics
[ Info: CUDAdrv.jl failed to initialize, GPU functionality unavailable (set JULIA_CUDA_SILENT or JULIA_CUDA_VERBOSE to silence or expand this message)
Completed tests for MoistThermodynamics, 30 seconds elapsed
Starting tests for PlanetParameters
Completed tests for PlanetParameters, 0 seconds elapsed
Completed tests for Common, 31 seconds elapsed
Test Summary: | Pass  Total
CLIMA         |  457    457
```

# For review by CLIMA developers

- [x] There are no open pull requests for this already
- [x] CLIMA developers with relevant expertise have been assigned to review this submission
- [ ] The code conforms to the [style guidelines](https://climate-machine.github.io/CLIMA/latest/CodingConventions.html) and has consistent naming conventions
- [ ] This code does what it is technically intended to do (all numerics make sense physically and/or computationally)


Co-authored-by: Charles Kawczynski <kawczynski.charles@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Test specific groups of tests testsets argument to Pkg.test() Pkg test with arguments for testing packages

6 participants