Switch to github actions #408
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,101 @@ | ||
| name: CI | ||
| on: | ||
| pull_request: | ||
| branches: | ||
| - master | ||
| push: | ||
| branches: | ||
| - master | ||
| tags: | ||
| - '*' | ||
| jobs: | ||
| tests-required: # Tests that needs to succeed | ||
| name: Julia ${{ matrix.version }} - ${{ matrix.os }} - ${{ matrix.arch }} - | ||
| runs-on: ${{ matrix.os }} | ||
| strategy: | ||
| fail-fast: false | ||
| matrix: | ||
| version: | ||
| - '1.0' # Replace this with the minimum Julia version that your package supports. E.g. if your package requires Julia 1.5 or higher, change this to '1.5'. | ||
| - '1' # Leave this line unchanged. '1' will automatically expand to the latest stable 1.x release of Julia. | ||
| os: | ||
| - ubuntu-latest | ||
| arch: | ||
| - x64 | ||
| steps: | ||
| - uses: actions/checkout@v2 | ||
| - uses: julia-actions/setup-julia@v1 | ||
| with: | ||
| version: ${{ matrix.version }} | ||
| arch: ${{ matrix.arch }} | ||
| - uses: actions/cache@v1 | ||
| env: | ||
| cache-name: cache-artifacts | ||
| with: | ||
| path: ~/.julia/artifacts | ||
| key: ${{ runner.os }}-test-${{ env.cache-name }}-${{ hashFiles('**/Project.toml') }} | ||
| restore-keys: | | ||
| ${{ runner.os }}-test-${{ env.cache-name }}- | ||
| ${{ runner.os }}-test- | ||
| ${{ runner.os }}- | ||
| - uses: julia-actions/julia-buildpkg@v1 | ||
| - uses: julia-actions/julia-runtest@v1 | ||
| - uses: julia-actions/julia-processcoverage@v1 | ||
| - uses: codecov/codecov-action@v1 | ||
| with: | ||
| file: lcov.info | ||
| test-allowed-fail: # Tests that are allowed to fail | ||
| name: Julia ${{ matrix.version }} - ${{ matrix.os }} - ${{ matrix.arch }} - [allowed to fail] - | ||
| runs-on: ${{ matrix.os }} | ||
| continue-on-error: true | ||
| strategy: | ||
| fail-fast: false | ||
| matrix: | ||
| version: | ||
| - 'nightly' | ||
| os: | ||
| - ubuntu-latest | ||
| arch: | ||
| - x64 | ||
| steps: | ||
| - uses: actions/checkout@v2 | ||
| - uses: julia-actions/setup-julia@v1 | ||
| with: | ||
| version: ${{ matrix.version }} | ||
| arch: ${{ matrix.arch }} | ||
| - uses: actions/cache@v1 | ||
| env: | ||
| cache-name: cache-artifacts | ||
| with: | ||
| path: ~/.julia/artifacts | ||
| key: ${{ runner.os }}-test-${{ env.cache-name }}-${{ hashFiles('**/Project.toml') }} | ||
| restore-keys: | | ||
| ${{ runner.os }}-test-${{ env.cache-name }}- | ||
| ${{ runner.os }}-test- | ||
| ${{ runner.os }}- | ||
| - uses: julia-actions/julia-buildpkg@v1 | ||
| - uses: julia-actions/julia-runtest@v1 | ||
| docs: | ||
| name: Documentation | ||
| runs-on: ubuntu-latest | ||
| steps: | ||
| - uses: actions/checkout@v2 | ||
| - uses: julia-actions/setup-julia@v1 | ||
| with: | ||
| version: '1' | ||
| - name: Install apt deps | ||
| run: sudo apt-get install libgtk-3-dev dvipng texlive | ||
| - run: | | ||
| julia --project=docs -e ' | ||
| using Pkg | ||
| Pkg.develop(PackageSpec(path=pwd())) | ||
| Pkg.instantiate()' | ||
| - run: | | ||
| julia --project=docs -e ' | ||
| using Documenter: doctest | ||
| using ControlSystems | ||
| doctest(ControlSystems)' | ||
| - run: julia --project=docs docs/make.jl | ||
| env: | ||
| GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} | ||
| DOCUMENTER_KEY: ${{ secrets.DOCUMENTER_KEY }} | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,6 +1,6 @@ | ||
| # Set plot globals | ||
| ENV["PLOTS_TEST"] = "true" | ||
| ENV["GKSwstype"] = "nul" | ||
|
There was a problem hiding this comment. As suggested in #401 to remove dependency on xvfb. Did it here so I would not have to figure out how to get that working correctly in gh actions just to then have to undo it. If we encounter problem with generating images for the plots we can patch it later. |
||
|
|
||
| using Documenter, ControlSystems, Plots, LinearAlgebra, DSP | ||
| import GR # Bug with world age in Plots.jl, see https://github.com/JuliaPlots/Plots.jl/issues/1047 | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,6 +1,5 @@ | ||
| using ControlSystems | ||
| using Test, LinearAlgebra, Random | ||
| import Base.isapprox # In framework and test_synthesis | ||
| import SparseArrays: sparse # In test_matrix_comps | ||
| import DSP: conv # In test_conversion and test_synthesis | ||
|
|
@@ -39,10 +38,4 @@ my_tests = [ | |
| _t0 = time() | ||
| run_tests(my_tests) | ||
| println("Ran all code tests in $(round(time()-_t0, digits=2)) seconds") | ||
|
Comment on lines
-44
to
-47
There was a problem hiding this comment. Seems that not having doctest in with the other tests so that it is run when checking coverage reduce coverage quite a bit. There was a problem hiding this comment. Can you pinpoint which parts are getting reduced coverage when the doctests are removed? I don't think we should be relying on the doctests for the testing coverage, since I believe doctests are more designed to ensure the documentation examples don't go stale rather than the code is functionally correct. There was a problem hiding this comment. I agree, but historically we have had problems with where there docs didn't build properly or were outdated without us noticing, that is why I added it to the tests as well. If GitHub actions allows us to visualize and stop a merge when doctest fails I have no problem removing them from the tests, if we also add tests to cover the same functionality. Edit: A nice feature with having the doctest in tests if that wont forget to run the doctest when developing locally. I don't know about you, but I rarely run them manually when developing. There was a problem hiding this comment.
In the coverage report you can see exactly which lines lost coverage. Most of it seems to be from pid_design.jl and discrete.jl There was a problem hiding this comment.
We can have a step in the workflow that is running
I'd never run doctest locally if it was not in, though I'm not sure I'd want to most of the time. It feels like the tests are what I'd use to see if I'm close to something working, and the running the PR can catch if something broke with the docs, a little less time running locally for me 😄 But if you feel it's warranted to run doctest all the time that is also fine by me, you have more experience with the workflows for this package. There was a problem hiding this comment. The integration of github actions is (non surprising) much better than it ever was for Travis, and a commit or PR will display the red cross instead of the green checkbox whenever any of the worflows failed. It should thus be fairly easy to detect the docs build failing even if it's not part of the tests. There are some noise left due to coverage sometimes reporting a failure for no good reason, but the docs failure remains easily visible on hover over the red cross. There was a problem hiding this comment. We had some weird behaviour where makedocs in docs/make.jl failed the doctest (it runs doctest internally, and it printed the errors we had based on the version problems) but managed to do the build and the workflow counted as passed. This was not expected to me and is not documented as far as I can see in Documenter.jl, so for now I run doctest first by itself once and then run docs/make.jl where makedocs runs doctest a second time (this could be disabled for the makedocs). |
||
| end | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,6 +1,6 @@ | ||
| # Set plot globals | ||
| ENV["PLOTS_TEST"] = "true" | ||
| ENV["GKSwstype"] = "nul" | ||
|
|
||
| using Plots | ||
| gr() | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we want to run coverage for both 1.0 and 1? What does that even mean? Does it average over the two? Does it pick the one who reported first/last?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
IMO coverage is probably only needed on one version, unless there starts to be a lot of version-specific code introduced (which I don't forsee being an issue in this package).