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

Remove unneeded doctest filters and fix tests #768

Open
penelopeysm opened this issue Dec 21, 2024 · 0 comments · May be fixed by #769
Open

Remove unneeded doctest filters and fix tests #768

penelopeysm opened this issue Dec 21, 2024 · 0 comments · May be fixed by #769

Comments

@penelopeysm
Copy link
Member

penelopeysm commented Dec 21, 2024

doctestfilters = [
# Older versions will show "0 element Array" instead of "Type[]".
r"(Any\[\]|0-element Array{.+,[0-9]+})",
# Older versions will show "Array{...,1}" instead of "Vector{...}".
r"(Array{.+,\s?1}|Vector{.+})",
# Older versions will show "Array{...,2}" instead of "Matrix{...}".
r"(Array{.+,\s?2}|Matrix{.+})",
# Errors from macros sometimes result in `LoadError: LoadError:`
# rather than `LoadError:`, depending on Julia version.
r"ERROR: (LoadError:\s)+",
# Older versions do not have `;;]` but instead just `]` at end of the line
# => need to treat `;;]` and `]` as the same, i.e. ignore them if at the end of a line
r"(;;){0,1}\]$"m,
# Ignore the source of a warning in the doctest output, since this is dependent on host.
# This is a line that starts with "└ @ " and ends with the line number.
r"└ @ .+:[0-9]+",
]
doctest(DynamicPPL; manual=false, doctestfilters=doctestfilters)

Some of these are probably outdated, given that our Julia compat is now at 1.10. But more saliently: the filters are probably not doing what we might like them to do (at least with the current version of Documenter). See https://documenter.juliadocs.org/stable/man/doctests/#Filtering-Doctests

To use a single example,

# Older versions will show "Array{...,1}" instead of "Vector{...}".
r"(Array{.+,\s?1}|Vector{.+})",

What we probably want here is to replace Array{...,1} with Vector{...} when performing the doctests, so as per the Documenter docs, I think the filter should be

r"(Array{(.+),\s?1}" => s"Vector{\1})",

Instead, what the current filter does is to simply remove any lines which match the regex. This means any line of output in the doctests that contains either Array{...,1} or Vector{...} is simply ignored, which leads to false test successes.

Although note that the regex+substitution pair can't be specified as part of the call to doctest(). They have to be specified for each test individually. JuliaDocs/Documenter.jl#2360

@penelopeysm penelopeysm linked a pull request Dec 21, 2024 that will close this issue
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 a pull request may close this issue.

1 participant