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

Turn off doctests while building docs #1915

Merged
merged 5 commits into from Mar 24, 2022

Conversation

Saransh-cpp
Copy link
Member

@Saransh-cpp Saransh-cpp commented Mar 21, 2022

Fixes #1914

Now the doctests run only in the workflows, and only on Julia 1.6!

PR Checklist

  • Tests are added
  • Entry in NEWS.md
  • Documentation, if applicable

@ToucheSir
Copy link
Member

Oddly, https://github.com/FluxML/Flux.jl/runs/5631279988?check_suite_focus=true does not show "Docs" as a sub-suite. Can you confirm those tests are running at all?

@Saransh-cpp
Copy link
Member Author

Oddly, https://github.com/FluxML/Flux.jl/runs/5631279988?check_suite_focus=true does not show "Docs" as a sub-suite. Can you confirm those tests are running at all?

I think "docs" is an independent job. You can see the job running here - https://github.com/FluxML/Flux.jl/actions/runs/2017534829 - but not within the "test" job.

@Saransh-cpp
Copy link
Member Author

Oh yes, "docs" is not under "test", rather a separate job -

docs:
name: Documentation
runs-on: ubuntu-latest
steps:

@codecov-commenter
Copy link

codecov-commenter commented Mar 22, 2022

Codecov Report

Merging #1915 (dcc6df8) into master (3c935cc) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master    #1915   +/-   ##
=======================================
  Coverage   86.42%   86.42%           
=======================================
  Files          18       18           
  Lines        1422     1422           
=======================================
  Hits         1229     1229           
  Misses        193      193           

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 3c935cc...dcc6df8. Read the comment docs.

@ToucheSir
Copy link
Member

I was referring to

Flux.jl/test/runtests.jl

Lines 60 to 66 in 7b56813

@static if VERSION == v"1.6"
using Documenter
@testset "Docs" begin
DocMeta.setdocmeta!(Flux, :DocTestSetup, :(using Flux); recursive=true)
doctest(Flux)
end
end
, which would appear under the output of the test CI action step.

@Saransh-cpp
Copy link
Member Author

Oh, I completely missed this block. This test block is currently not running on the master branch as well (for example, notice how this block does not run on this (https://github.com/FluxML/Flux.jl/runs/5632762607?check_suite_focus=true) PR as well).

This is due to the fact that Julia's version is pinned to v1.6 in the code and the workflows pull in the latest Julia 1.6 version, that is v1.6.5.

Plus the repository has a separate workflow for doctesting, and I think running them in the test suite again would be redundant. Should I remove this block too?

@ToucheSir
Copy link
Member

That's fine, but are we sure the doctests are running on a docs build given https://github.com/FluxML/Flux.jl/blob/master/docs/make.jl#L5?

@Saransh-cpp
Copy link
Member Author

Yes! The tests are running under the CI / Documentation workflow (as they were running before). For this PR - https://github.com/FluxML/Flux.jl/runs/5639377626?check_suite_focus=true

image

@ToucheSir
Copy link
Member

Why is it showing only 1 test? Can you tweak a doctest to be failing and confirm that it doesn't pass locally?

@Saransh-cpp
Copy link
Member Author

Saransh-cpp commented Mar 23, 2022

The doctests do fail when they are written wrong -

julia> using Flux
[ Info: Precompiling Flux [587475ba-b771-5e3f-ad9e-33799f191a9c]

julia> using Documenter

julia> using Documenter: doctest

julia> DocMeta.setdocmeta!(Flux, :DocTestSetup, :(using Flux); recursive=true)

julia> doctest(Flux)
[ Info: SetupBuildDirectory: setting up build directory.
[ Info: Doctest: running doctests.
┌ Error: doctest failure in C:\Users\Saransh\.julia\dev\Flux.jl\src\layers\conv.jl:74-94
│ 
│ ```jldoctest
│ julia> xs = rand(Float32, 100, 100, 3, 50); # a batch of images

│ julia> layer = Conv((5,5), 3 => 7, relu; bias = false)
│ Conv((5, 5), 3 => 7, relu, bias=false)  # 525 parameters

│ julia> layer(xs) |> size
│ (96, 96, 7, 50)

│ julia> Conv((5,5), 3 => 7; stride = 2)(xs) |> size
│ (48, 48, 7, 50)

│ julia> Conv((5,5), 3 => 7; stride = 2, pad = SamePad())(xs) |> size
│ (50, 50, 7, 50)

│ julia> Conv((1,1), 3 => 7; pad = (20,10,0,0))(xs) |> size
│ (130, 100, 7, 50)

│ julia> Conv((5,5), 3 => 7; stride = 2, dilation = 4)(xs) |> size
│ (42, 42, 7, 50
```
│ 
│ Subexpression:
│ 
│ Conv((5,5), 3 => 7; stride = 2, dilation = 4)(xs) |> size
│ 
│ Evaluated output:
│ 
│ (42, 42, 7, 50)
│ 
│ Expected output:
│ 
│ (42, 42, 7, 50
│ 
│   diff = (42, 42, 7, 5050)
└ @ Documenter.DocTests C:\Users\Saransh\.julia\dev\Flux.jl\src\layers\conv.jl:74
┌ Error: Doctesting failed
│   exception =`makedocs` encountered a doctest error. Terminating build
│    Stacktrace:
│      [1] error(s::String)
│        @ Base .\error.jl:33
│      [2] runner(#unused#::Type{Documenter.Builder.Doctest}, doc::Documenter.Documents.Document)
│        @ Documenter.Builder C:\Users\Saransh\.julia\packages\Documenter\7hBIS\src\Builder.jl:216
│      [3] dispatch(#unused#::Type{Documenter.Builder.DocumentPipeline}, x::Documenter.Documents.Document)
│        @ Documenter.Utilities.Selectors C:\Users\Saransh\.julia\packages\Documenter\7hBIS\src\Utilities\Selectors.jl:170
│      [4] #2
│        @ C:\Users\Saransh\.julia\packages\Documenter\7hBIS\src\Documenter.jl:266 [inlined]
│      [5] cd(f::Documenter.var"#2#3"{Documenter.Documents.Document}, dir::String)
│        @ Base.Filesystem .\file.jl:95
│      [6] makedocs(; debug::Bool, format::Documenter.Writers.HTMLWriter.HTML, kwargs::Base.Iterators.Pairs{Symbol, Any, NTuple{6, Symbol}, NamedTuple{(:root, :source, :sitename, :doctest, :modules, :doctestfilters), Tuple{String, String, String, Symbol, Vector{Module}, Vector{Regex}}}})
│        @ Documenter C:\Users\Saransh\.julia\packages\Documenter\7hBIS\src\Documenter.jl:265
│      [7] (::Documenter.var"#all_doctests#32"{Bool, Vector{Regex}, Vector{Module}})()
│        @ Documenter C:\Users\Saransh\.julia\packages\Documenter\7hBIS\src\Documenter.jl:894
│      [8] macro expansion
│        @ C:\Users\Saransh\.julia\packages\Documenter\7hBIS\src\Documenter.jl:915 [inlined]
│      [9] macro expansion
│        @ C:\buildbot\worker\package_win64\build\usr\share\julia\stdlib\v1.6\Test\src\Test.jl:1151 [inlined]
│     [10] doctest(source::String, modules::Vector{Module}; fix::Bool, testset::String, doctestfilters::Vector{Regex})
│        @ Documenter C:\Users\Saransh\.julia\packages\Documenter\7hBIS\src\Documenter.jl:915
│     [11] doctest(package::Module; manual::Bool, testset::Nothing, kwargs::Base.Iterators.Pairs{Union{}, Union{}, Tuple{}, NamedTuple{(), Tuple{}}})
│        @ Documenter C:\Users\Saransh\.julia\packages\Documenter\7hBIS\src\Documenter.jl:850
│     [12] doctest(package::Module)
│        @ Documenter C:\Users\Saransh\.julia\packages\Documenter\7hBIS\src\Documenter.jl:837
│     [13] top-level scope
│        @ REPL[5]:1
│     [14] top-level scope
│        @ C:\Users\Saransh\.julia\packages\CUDA\5jdFl\src\initialization.jl:52
│     [15] eval
│        @ .\boot.jl:360 [inlined]
│     [16] eval_user_input(ast::Any, backend::REPL.REPLBackend)
│        @ REPL C:\buildbot\worker\package_win64\build\usr\share\julia\stdlib\v1.6\REPL\src\REPL.jl:139
│     [17] repl_backend_loop(backend::REPL.REPLBackend)
│        @ REPL C:\buildbot\worker\package_win64\build\usr\share\julia\stdlib\v1.6\REPL\src\REPL.jl:200
│     [18] start_repl_backend(backend::REPL.REPLBackend, consumer::Any)
│        @ REPL C:\buildbot\worker\package_win64\build\usr\share\julia\stdlib\v1.6\REPL\src\REPL.jl:185
│     [19] run_repl(repl::REPL.AbstractREPL, consumer::Any; backend_on_current_task::Bool)
│        @ REPL C:\buildbot\worker\package_win64\build\usr\share\julia\stdlib\v1.6\REPL\src\REPL.jl:317
│     [20] run_repl(repl::REPL.AbstractREPL, consumer::Any)
│        @ REPL C:\buildbot\worker\package_win64\build\usr\share\julia\stdlib\v1.6\REPL\src\REPL.jl:305
│     [21] (::Base.var"#875#877"{Bool, Bool, Bool})(REPL::Module)
│        @ Base .\client.jl:387
│     [22] #invokelatest#2
│        @ .\essentials.jl:708 [inlined]
│     [23] invokelatest
│        @ .\essentials.jl:706 [inlined]
│     [24] run_main_repl(interactive::Bool, quiet::Bool, banner::Bool, history_file::Bool, color_set::Bool)
│        @ Base .\client.jl:372
│     [25] exec_options(opts::Base.JLOptions)
│        @ Base .\client.jl:302
│     [26] _start()
│        @ Base .\client.jl:485
└ @ Documenter C:\Users\Saransh\.julia\packages\Documenter\7hBIS\src\Documenter.jl:904
Doctests: Flux: Test Failed at C:\Users\Saransh\.julia\packages\Documenter\7hBIS\src\Documenter.jl:915
  Expression: all_doctests()
Stacktrace:
 [1] macro expansion
   @ C:\Users\Saransh\.julia\packages\Documenter\7hBIS\src\Documenter.jl:915 [inlined]
 [2] macro expansion
   @ C:\buildbot\worker\package_win64\build\usr\share\julia\stdlib\v1.6\Test\src\Test.jl:1151 [inlined]
 [3] doctest(source::String, modules::Vector{Module}; fix::Bool, testset::String, doctestfilters::Vector{Regex})
   @ Documenter C:\Users\Saransh\.julia\packages\Documenter\7hBIS\src\Documenter.jl:915
Test Summary:  | Fail  Total
Doctests: Flux |    1      1
ERROR: Some tests did not pass: 0 passed, 1 failed, 0 errored, 0 broken.

I removed a ) from the output. This should also fail on the latest commit of this PR but I think GitHub Actions are down at the moment :(

@ToucheSir
Copy link
Member

Ok, LMK once you've reverted the ) removal and we can get this merged.

@Saransh-cpp
Copy link
Member Author

Done!

@ToucheSir ToucheSir merged commit a851436 into FluxML:master Mar 24, 2022
@Saransh-cpp Saransh-cpp deleted the fix-julia-version-for-doctest branch March 25, 2022 03:31
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.

Different Julia versions at different places for doctests
3 participants