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

@debug the next REPL line before evaling it #2179

Merged
merged 4 commits into from
Jul 13, 2023

Conversation

musoke
Copy link
Contributor

@musoke musoke commented Jul 11, 2023

Add a @debug expression within DocTests.eval_repl to show which line is about to be evaluated.
This was requested in #2013

I have found it useful while attempting to track down a similar issue.

@musoke
Copy link
Contributor Author

musoke commented Jul 11, 2023

Apologies to user @ debug for accidentally tagging you on this.

@mortenpi
Copy link
Member

mortenpi commented Jul 12, 2023

Do you have an example of what the output looks from this?

@musoke
Copy link
Contributor Author

musoke commented Jul 12, 2023

I do. Here is output I got while debugging the doc builds of musoke/WolframExpr.jl@30e0a5b.

In this case one can get some hints from looking at the debug log of the previous line of the REPL, but it is not clear if the hang happened during verification of the previous output or creation of the next. It's also not helpful if the error is on the first line.

     Testing Running tests...
┌ Debug: Doctesting in temporary directory: /tmp/jl_4fcV7O
│   modules =
│    1-element Vector{Module}:
│     WolframExpr
└ @ Documenter ~/.julia/packages/Documenter/oxl3Q/src/doctest.jl:73
┌ Debug: Document: remotes
│   remote = nothing
│   remotes = nothing
└ @ Documenter ~/.julia/packages/Documenter/oxl3Q/src/documents.jl:380
[ Info: SetupBuildDirectory: setting up build directory.
[ Info: Doctest: running doctests.
┌ Debug: Running doctests.
└ @ Documenter.DocTests ~/.julia/packages/Documenter/oxl3Q/src/DocTests.jl:47
┌ Debug: Evaluating REPL line
│   input = "using WolframExpr"
│   output = ""
└ @ Documenter.DocTests ~/.julia/packages/Documenter/oxl3Q/src/DocTests.jl:244
┌ Debug: Verifying doctest at ~/.julia/dev/WolframExpr/src/WolframExpr.jl:20-30
│
│ ```jldoctest
│ julia> using WolframExpr
│
│ julia> f = string_to_function("x+y", [:x, :y]);
│
│ julia> f(1, 2)
│ 3
│
│ julia> f(1.2, 2)
│ 3.2
│ ```
│
│ Subexpression:
│
│ using WolframExpr
│
│ Evaluated output:
│
│
│
│ Expected output:
│
│
└ @ Documenter.DocTests ~/.julia/packages/Documenter/oxl3Q/src/DocTests.jl:351
┌ Debug: Evaluating REPL line
│   input = "f = string_to_function(\"x+y\", [:x, :y]);"
│   output = ""
└ @ Documenter.DocTests ~/.julia/packages/Documenter/oxl3Q/src/DocTests.jl:244

Copy link
Member

@mortenpi mortenpi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure if it become too noisy or not, but if you think more detail would be useful, then the at-debug could also go into the inner for?

src/DocTests.jl Outdated
@@ -241,6 +241,7 @@ end

function eval_repl(block, sandbox, meta::Dict, doc::Documenter.Document, page)
for (input, output) in repl_splitter(block.code)
@debug "Evaluating REPL line" input output
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
@debug "Evaluating REPL line" input output
@debug "Evaluating doctest REPL line" input expected_output = output

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I updated to include this suggestion and the file & line numbers of the doctest. The output for the same test as above now looks like

     Testing Running tests...
     ┌ Debug: Doctesting in temporary directory: /tmp/jl_58qp0Y
     │   modules =
     │    1-element Vector{Module}:
     │     WolframExpr
     └ @ Documenter ~/.julia/dev/Documenter/src/doctest.jl:73
     ┌ Debug: Document: remotes
     │   remote = nothing
     │   remotes = nothing
     └ @ Documenter ~/.julia/dev/Documenter/src/documents.jl:380
     [ Info: SetupBuildDirectory: setting up build directory.
     [ Info: Doctest: running doctests.
     ┌ Debug: Running doctests.
     └ @ Documenter.DocTests ~/.julia/dev/Documenter/src/DocTests.jl:47
     ┌ Debug: Evaluating doctest REPL line from ~/.julia/dev/WolframExpr/src/WolframExpr.jl:20-30
     │   input = "using WolframExpr"
     │   expected_output = ""
     └ @ Documenter.DocTests ~/.julia/dev/Documenter/src/DocTests.jl:246
     ┌ Debug: Verifying doctest at ~/.julia/dev/WolframExpr/src/WolframExpr.jl:20-30
     │
     │ ```jldoctest
     │ julia> using WolframExpr
     │
     │ julia> f = string_to_function("x+y", [:x, :y]);
     │
     │ julia> f(1, 2)
     │ 3
     │
     │ julia> f(1.2, 2)
     │ 3.2
     │ ```
     │
     │ Subexpression:
     │
     │ using WolframExpr
     │
     │ Evaluated output:
     │
     │
     │
     │ Expected output:
     │
     │
     └ @ Documenter.DocTests ~/.julia/dev/Documenter/src/DocTests.jl:354
     ┌ Debug: Evaluating doctest REPL line from ~/.julia/dev/WolframExpr/src/WolframExpr.jl:20-30
     │   input = "f = string_to_function(\"x+y\", [:x, :y]);"
     │   expected_output = ""
     └ @ Documenter.DocTests ~/.julia/dev/Documenter/src/DocTests.jl:246

Add a `@debug` expression within DocTests.eval_repl to show which line is
about to be evaluated.
This was requested in
JuliaDocs#2013
src/DocTests.jl Outdated
@@ -242,6 +242,8 @@ end
function eval_repl(block, sandbox, meta::Dict, doc::Documenter.Document, page)
for (input, output) in repl_splitter(block.code)
result = Result(block, input, output, meta[:CurrentFile])
src_lines = Documenter.find_block_in_file(result.block.code, result.file)
Copy link
Contributor Author

@musoke musoke Jul 12, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ideally, this would be more like

src_lines = Documenter.find_block_in_file(input, result.file)

to narrow down the exact line, but find_block_in_file finds the first match in the file, so lines that appear in more than one doctest are wrong. E.g. initial using statements.

As written, there may still be slight confusion if the same line appears twice in one doctest or major confusion if one doctest appears as a subset of another.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

result.block should be the same as block, so maybe we can move this outside of the for loop? In other words, it's looking for the line range of the whole jldoctest code block anyway, right?

@musoke
Copy link
Contributor Author

musoke commented Jul 12, 2023

Not sure if it become too noisy or not, but if you think more detail would be useful, then the at-debug could also go into the inner for?

I had this in an earlier version but didn't include it the initial PR. It is no noisier when each REPL line contains only one expression. (This also means that I didn't need it for what I was testing, so wasn't sure how useful it is.)

I have pushed a version that does this. The exs in the inner loop can't be matched to expected output, so I don't output them. They can be seen in the output of debug_report if it gets far enough to matter.

Here's updated output:

     Testing Running tests...
     ┌ Debug: Doctesting in temporary directory: /tmp/jl_Syps7H
     │   modules =
     │    1-element Vector{Module}:
     │     WolframExpr
     └ @ Documenter ~/.julia/dev/Documenter/src/doctest.jl:73
     ┌ Debug: Document: remotes
     │   remote = nothing
     │   remotes = nothing
     └ @ Documenter ~/.julia/dev/Documenter/src/documents.jl:380
     [ Info: SetupBuildDirectory: setting up build directory.
     [ Info: Doctest: running doctests.
     ┌ Debug: Running doctests.
     └ @ Documenter.DocTests ~/.julia/dev/Documenter/src/DocTests.jl:47
     ┌ Debug: Evaluating doctest REPL line from ~/.julia/dev/WolframExpr/src/WolframExpr.jl:20-30
     │   unparsed_string = "using WolframExpr\n"
     │   parsed_expression = :(using WolframExpr)
     └ @ Documenter.DocTests ~/.julia/dev/Documenter/src/DocTests.jl:248
     ┌ Debug: Verifying doctest at ~/.julia/dev/WolframExpr/src/WolframExpr.jl:20-30
     │
     │ ```jldoctest
     │ julia> using WolframExpr
     │
     │ julia> f = string_to_function("x+y", [:x, :y]);
     │
     │ julia> f(1, 2)
     │ 3
     │
     │ julia> f(1.2, 2)
     │ 3.2
     │ ```
     │
     │ Subexpression:
     │
     │ using WolframExpr
     │
     │ Evaluated output:
     │
     │
     │
     │ Expected output:
     │
     │
     └ @ Documenter.DocTests ~/.julia/dev/Documenter/src/DocTests.jl:353
     ┌ Debug: Evaluating doctest REPL line from ~/.julia/dev/WolframExpr/src/WolframExpr.jl:20-30
     │   unparsed_string = "f = string_to_function(\"x+y\", [:x, :y]);\n"
     │   parsed_expression = :($(Expr(:toplevel, :(f = string_to_function("x+y", [:x, :y])))))
     └ @ Documenter.DocTests ~/.julia/dev/Documenter/src/DocTests.jl:248

src/DocTests.jl Outdated Show resolved Hide resolved
@mortenpi
Copy link
Member

I had this in an earlier version but didn't include it the initial PR. It is no noisier when each REPL line contains only one expression. (This also means that I didn't need it for what I was testing, so wasn't sure how useful it is.)

Let's keep it though. Being closer to what Documenter does internally is probably good when debugging. And we can always change this later if it turns out to be a problem 🙂

@musoke
Copy link
Contributor Author

musoke commented Jul 13, 2023

Thanks! I have made the suggested changes

@mortenpi mortenpi enabled auto-merge (squash) July 13, 2023 23:10
Copy link
Member

@mortenpi mortenpi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks!

@mortenpi mortenpi merged commit b9f8c61 into JuliaDocs:master Jul 13, 2023
@musoke
Copy link
Contributor Author

musoke commented Jul 13, 2023

Thanks Morten! Now I'll get back to debugging the issue that started this.

@musoke musoke deleted the debug-hang branch July 13, 2023 23:22
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.

2 participants