-
Notifications
You must be signed in to change notification settings - Fork 86
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
Switch to github actions #408
Changes from all commits
ce1fd84
44ffd89
3b85c37
f01144d
70e6e1f
af814dd
4c5fd9b
ccd85d1
7314ee7
6e63f29
77de81e
30fe8f6
d4d25bb
ee76475
880f699
bffce77
ad622e2
b9ac0cf
bd2f0d5
4bd3459
89b004e
f7e3d05
f27ae78
2cf7067
1332438
c1c8e60
8a0403d
1861156
5680c1b
7920d07
1f55060
7d4bf93
f2bb4bd
c117d7a
1b5ef98
cc67124
d9b21b9
4772cc9
d49c1a2
d50784b
cdbfe20
4eb8a0d
f521f6c
53e82f9
8d677c6
cb960ef
58d0c64
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,58 @@ | ||
name: Docs | ||
|
||
on: | ||
pull_request: | ||
branches: | ||
- master | ||
push: | ||
branches: | ||
- master | ||
tags: | ||
- '*' | ||
|
||
jobs: | ||
docs: | ||
name: Julia ${{ matrix.version }} - ${{ matrix.os }} - ${{ matrix.arch }} - ${{ github.event_name }} | ||
runs-on: ${{ matrix.os }} | ||
timeout-minutes: 30 | ||
strategy: | ||
matrix: | ||
version: [1] | ||
os: [ubuntu-latest] | ||
arch: [x64] | ||
steps: | ||
- uses: actions/checkout@v2 | ||
- uses: julia-actions/setup-julia@v1 | ||
with: | ||
version: ${{ matrix.version }} | ||
arch: ${{ matrix.arch }} | ||
- name: Install apt deps | ||
run: sudo apt-get install libgtk-3-dev dvipng texlive | ||
- 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 }}- | ||
- name: Setup docs environment | ||
run: | | ||
julia --project=docs -e ' | ||
using Pkg | ||
Pkg.develop(PackageSpec(path=pwd())) | ||
Pkg.instantiate()' | ||
- name: Test docs # Test are also run in the makedocs, but there they don't error properly unless we set strict in which case warnings also error | ||
run: | | ||
julia --project=docs -e ' | ||
ENV["PLOTS_TEST"] = "true" | ||
ENV["GKSwstype"] = "nul" | ||
using Documenter: doctest | ||
using ControlSystems | ||
doctest(ControlSystems)' | ||
- name: Generate documentation | ||
run: julia --project=docs --color=yes docs/make.jl | ||
env: | ||
GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,48 @@ | ||
name: CI | ||
|
||
on: | ||
pull_request: | ||
branches: | ||
- master | ||
push: | ||
branches: | ||
- master | ||
tags: | ||
- '*' | ||
|
||
jobs: | ||
runtests: | ||
name: Julia ${{ matrix.version }} - ${{ matrix.os }} - ${{ matrix.arch }} - ${{ github.event_name }} | ||
runs-on: ${{ matrix.os }} | ||
continue-on-error: ${{ matrix.version == 'nightly' }} # Allow nightly to fail and workflow still count as completed | ||
timeout-minutes: 30 | ||
strategy: | ||
fail-fast: false | ||
matrix: | ||
version: ['1.0', '1', '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 | ||
- uses: julia-actions/julia-processcoverage@v1 | ||
if: ${{ matrix.version == '1' }} | ||
- uses: codecov/codecov-action@v1 | ||
if: ${{ matrix.version == '1' }} | ||
with: | ||
file: lcov.info |
This file was deleted.
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,6 +1,5 @@ | ||
using ControlSystems | ||
using Test, LinearAlgebra, Random | ||
using Documenter # For doctests | ||
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") | ||
|
||
|
||
println("Test Doctests") | ||
_t0 = time() | ||
doctest(ControlSystems) | ||
println("Ran Doctests in $(round(time()-_t0, digits=2)) seconds") | ||
Comment on lines
-44
to
-47
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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 |
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.
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.