#26017 closed defect (fixed)
Improve regression test on search_doc
Reported by: | embray | Owned by: | |
---|---|---|---|
Priority: | major | Milestone: | sage-8.4 |
Component: | misc | Keywords: | |
Cc: | Merged in: | ||
Authors: | Erik Bray | Reviewers: | Julian Rüth |
Report Upstream: | N/A | Work issues: | |
Branch: | 691cca7 (Commits, GitHub, GitLab) | Commit: | |
Dependencies: | Stopgaps: |
Description
This test started failing for me, due to there being more than 2000 matches for "tree" in my docs (even when considering whole word matches). The rewritten test demonstrates, I believe, that the failure is not in the code itself, but rather that my docs directory just started having a larger number of matches than the hard-coded threshold. I'm not sure why that is, and it's possible I need to do a make doc-clean
.
But nevertheless this test can still be rewritten to not depend on some arbitrary thresholds.
(Incidentally, I also noticed that this full text search over all the docs is very, very slow. Doesn't Sphinx generate some indices we could be taking advantage of...?)
Change History (19)
comment:1 Changed 4 years ago by
- Status changed from new to needs_review
comment:2 Changed 4 years ago by
- Commit changed from cad76698fa8c360586ad8c66dbc8fcd0fb9b9f08 to 48ba5ae36815afb34473ef5f28ee100baa2a7f91
comment:3 Changed 4 years ago by
- Reviewers set to Julian Rüth
- Status changed from needs_review to positive_review
comment:4 Changed 4 years ago by
FWIW running a make doc-clean
and rebuilding the docs in the source tree where I was getting this failure also fixed the failure of the original test, as I suspected it might. Nevertheless I think this rewrite makes more sense.
comment:6 Changed 4 years ago by
Dare I ask what with??
comment:7 Changed 4 years ago by
If you're going to write "merge conflict" you could at least produce the diff as a mere hint of what it might conflict with.
comment:8 Changed 4 years ago by
- Status changed from needs_work to needs_info
comment:9 Changed 4 years ago by
I guess that's just Volker's workflow. He merges things into his private "alpha" branch and reports merge conflicts that you won't see until the next beta. I think this kind of workflow is fine but the communication could be a bit more clear about this (as I have already been on quite a few tickets where this caused a bit of confusion.) Something like
The Release Manager could not merge the changes on this ticket because there were merge conflicts. These conflicts might not be visible to you until the next beta of Sage has been released. Please merge the develop branch into this ticket's branch once the next beta version of Sage has been released.
vbraun: What do you think?
comment:10 Changed 4 years ago by
I've already brought this up before and suggested that the "private" branch be more visible. The response to this has usually been along the lines of "it's not hidden--you can see it right here". But that's extremely non-obvious. I might know that (now) but it's a pretty confusing and unwelcoming response to new users. I have suggested that it be an easily locatable, well-known branch name on git.sagemath.org (I would prefer an integration branch for the next release or something like that) along with links to that branch so that I can see what's going on there.
The response to that has generally been along the lines of "we can't do that because it would be confusing if people base stuff off that branch". But I don't buy that. Even if someone did, so what? Just say that it doesn't merge and why, and that they should rebase on "develop" or something. I think in practice that would be pretty rare. Much less rare than telling that their ticket has a merge conflict--when by appearances it doesn't--and then not explaining what the conflict is. And then worse, setting the ticket back to "needs work", where it's possible it will be forgotten and decay further.
comment:11 Changed 4 years ago by
I tried to propose something (in May) :
comment:12 Changed 4 years ago by
See #26102 for a slight speed-up to search_doc
.
comment:13 Changed 4 years ago by
- Commit changed from 48ba5ae36815afb34473ef5f28ee100baa2a7f91 to 40c91b3e077c8f623f34e59295bd8bacdf424fc4
Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:
40c91b3 | rewrite this test to not depend on relatively arbitrary bounds
|
comment:14 Changed 4 years ago by
- Status changed from needs_info to positive_review
Rebased, I guess. Thanks for the info.
comment:15 Changed 4 years ago by
- Status changed from positive_review to needs_work
********************************************************************** File "src/sage/misc/sagedoc.py", line 1179, in sage.misc.sagedoc.? Failed example: len(L) < N # long time Exception raised: Traceback (most recent call last): File "/var/lib/buildbot/slave/sage_git/build/local/lib/python2.7/site-packages/sage/doctest/forker.py", line 650, in _run self.compile_and_execute(example, compiler, test.globs) File "/var/lib/buildbot/slave/sage_git/build/local/lib/python2.7/site-packages/sage/doctest/forker.py", line 1061, in compile_and_execute exec(compiled, globs) File "<doctest sage.misc.sagedoc.?[1]>", line 1, in <module> len(L) < N # long time NameError: name 'L' is not defined ********************************************************************** File "src/sage/misc/sagedoc.py", line 1183, in sage.misc.sagedoc.? Failed example: all(tree_re.search(l) for l in L) # long time Exception raised: Traceback (most recent call last): File "/var/lib/buildbot/slave/sage_git/build/local/lib/python2.7/site-packages/sage/doctest/forker.py", line 650, in _run self.compile_and_execute(example, compiler, test.globs) File "/var/lib/buildbot/slave/sage_git/build/local/lib/python2.7/site-packages/sage/doctest/forker.py", line 1061, in compile_and_execute exec(compiled, globs) File "<doctest sage.misc.sagedoc.?[4]>", line 1, in <module> all(tree_re.search(l) for l in L) # long time NameError: name 'L' is not defined **********************************************************************
comment:16 Changed 4 years ago by
- Branch changed from u/embray/misc/search_doc/doctest to u/jhpalmieri/misc/search_doc/doctest
comment:17 Changed 4 years ago by
- Commit changed from 40c91b3e077c8f623f34e59295bd8bacdf424fc4 to 691cca71282ee91e92d5d07bd49deaa447f03d10
- Status changed from needs_work to positive_review
Fixed the optional
tags on the doctests.
New commits:
691cca7 | trac 26017: fix optional tags on doctests
|
comment:18 Changed 4 years ago by
- Branch changed from u/jhpalmieri/misc/search_doc/doctest to 691cca71282ee91e92d5d07bd49deaa447f03d10
- Resolution set to fixed
- Status changed from positive_review to closed
comment:19 Changed 4 years ago by
- Commit 691cca71282ee91e92d5d07bd49deaa447f03d10 deleted
Breakage: #26184
Branch pushed to git repo; I updated commit sha1. New commits:
whitespace cleanup