#30410 closed enhancement (fixed)
Command "sage tox"
Reported by:  mkoeppe  Owned by:  

Priority:  major  Milestone:  sage9.2 
Component:  doctest framework  Keywords:  
Cc:  ghtobiasdiez, dimpase, jhpalmieri, chapoton  Merged in:  
Authors:  Matthias Koeppe  Reviewers:  Dima Pasechnik 
Report Upstream:  N/A  Work issues:  
Branch:  eba708b (Commits, GitHub, GitLab)  Commit:  
Dependencies:  #30408  Stopgaps: 
Description (last modified by )
This will run tox using src/tox.ini
.
Entry point for doctesting and linting: See ./sage tox l v
or (cd src && tox l v)
.
Change History (27)
comment:1 Changed 2 years ago by
Branch:  → u/mkoeppe/command__sage__tox_ 

comment:2 Changed 2 years ago by
Authors:  → Matthias Koeppe 

Commit:  → 5ccccbb0f7a28c878646fcf5c1580cba0e26c475 
Description:  modified (diff) 
Status:  new → needs_review 
comment:3 followup: 5 Changed 2 years ago by
Instead of putting more into the bin/sage
script, wouldn't it make sense to actually extract all building and testing related code to tox? So instead of sage b
(sage t
) you run tox build
(tox test
respectively). This would leave the sage
script only with the functionality that a user needs to run sage.
(also I don't see any advantages of sage tox pycodestyle
over tox pycodestyle
)
comment:4 Changed 2 years ago by
Dependencies:  → 30361 

Whatever the final interface looks like, the documentation needs to be changed as well to reflect this. In particular, this ticket depends on #30361.
comment:5 Changed 2 years ago by
Replying to ghtobiasdiez:
Instead of putting more into the
bin/sage
script, wouldn't it make sense to actually extract all building and testing related code to tox? So instead ofsage b
(sage t
) you runtox build
(tox test
respectively). This would leave thesage
script only with the functionality that a user needs to run sage.
Some projects are indeed using tox also for building, but I think I would like to keep it for testing only. At least for now  we can't make too many changes at the same time so that our developer community is not overwhelmed.
sage coverage
and sage startuptime
, however, should definitely become tox environments.
(also I don't see any advantages of
sage tox pycodestyle
overtox pycodestyle
)
The difference is that sage tox
would also work with src/tox.ini
when called from SAGE_ROOT. (There's another tox.ini in SAGE_ROOT for a different purpose  testing of the sage distribution.)
And it can give a hint to install tox when it is not available.
comment:6 Changed 2 years ago by
Dependencies:  30361 → #30408, #30361 

comment:7 Changed 2 years ago by
Commit:  5ccccbb0f7a28c878646fcf5c1580cba0e26c475 → 857444889f93a3053dda3e195444b3560b35ad62 

Branch pushed to git repo; I updated commit sha1. New commits:
1cedc00  Add configuration of pycodestyle via tox

dde3b4f  src/tox.ini: Add testenv:pycodestyle

939e9e0  src/tox.ini: Better default arg testenv:pycodestyle

583bde5  Merge branch 't/30408/public/build/pycodestyleConfig' into t/30410/command__sage__tox_

8574448  src/tox.ini: Add coverage, startuptime; refactor, add descriptions

comment:8 Changed 2 years ago by
Dependencies:  #30408, #30361 → #30408 

Description:  modified (diff) 
I don't think #30361 needs to be a dependency.
comment:9 followup: 11 Changed 2 years ago by
It should also be possible to add simple forwards in the tox.ini
at the root folder of the form
[testenv:pycodestyle] whitelist_externals=tox commands=tox c {toxinidir}/src/tox.ini e pycodestyle
Then you can simply call tox e pycodestyle
.
comment:10 followup: 12 Changed 2 years ago by
I also think the testing methods in bin/sage should now be removed or deprecated (or at least use tox), or not?
comment:11 Changed 2 years ago by
Replying to ghtobiasdiez:
It should also be possible to add simple forwards in the
tox.ini
at the root folder
Good idea, will do.
comment:12 followup: 13 Changed 2 years ago by
Replying to ghtobiasdiez:
I also think the testing methods in bin/sage should now be removed or deprecated (or at least use tox), or not?
For that we would first have to make tox a "standard" package. Step by step...
comment:13 Changed 2 years ago by
Replying to mkoeppe:
Replying to ghtobiasdiez:
I also think the testing methods in bin/sage should now be removed or deprecated (or at least use tox), or not?
For that we would first have to make tox a "standard" package. Step by step...
Opened #30416 for that.
comment:14 Changed 2 years ago by
Commit:  857444889f93a3053dda3e195444b3560b35ad62 → eba708b89b9935f86d6db8369ef83bcfa1ca9d4b 

Branch pushed to git repo; I updated commit sha1. New commits:
eba708b  tox.ini: Delegate doctest, coverage, startuptime, pycodestyle to src/tox.ini

comment:16 followup: 17 Changed 2 years ago by
Looks good to me.
The only point where I'm not sure is whether the src/tox.ini file is really needed, or if it can be combined with the tox.ini in the root directory. Similarly, I'm not sure if sage tox should always be relative to the tox config in the src folder.
comment:17 Changed 2 years ago by
Replying to ghtobiasdiez:
The only point where I'm not sure is whether the src/tox.ini file is really needed, or if it can be combined with the tox.ini in the root directory.
Yes, I have been debating this with myself actually, but in the end I think it is important to keep src/tox.ini simple and readable  the portability testing stuff in the root can be overwhelming to developers.
Similarly, I'm not sure if sage tox should always be relative to the tox config in the src folder.
The purpose of this is to make it easy to run the tests on user code  in the same way that sage t
is often used.
comment:18 followup: 19 Changed 2 years ago by
I've tried it on a file (with systemwide tox and python):
sagedev/src $ ../sage tox sage/graphs/generators/distance_regular.pyx doctest runtestpre: PYTHONHASHSEED='2313650994' doctest runtest: commands[0]  sh c '/mnt/opt/Sage/sagedev/src/../sage t p 0 sage/graphs/generators/distance_regular.pyx' Running doctests with ID 20200827172003d333763b. Git branch: HEAD Using optional=build,dochtml,e_antic,gap_packages,libsemigroups,meataxe,memlimit,normaliz,pynormaliz,sage Doctesting 1 file using 4 threads. sage t warnlong 108.5 randomseed=0 sage/graphs/generators/distance_regular.pyx [123 tests, 172.32 s]  All tests passed!  Total time for all tests: 172.4 seconds cpu time: 172.5 seconds cumulative wall time: 172.3 seconds coverage runtestpre: PYTHONHASHSEED='2313650994' coverage runtest: commands[0]  sh c 'if [ z "sage/graphs/generators/distance_regular.pyx" ]; then /mnt/opt/Sage/sagedev/src/../sage coverageall; else /mnt/opt/Sage/sagedev/src/../sage coverage sage/graphs/generators/distance_regular.pyx; fi'  SCORE sage/graphs/generators/distance_regular.pyx: 100.0% (30 of 30)  startuptime runtestpre: PYTHONHASHSEED='2313650994' startuptime runtest: commands[0]  sh c '/mnt/opt/Sage/sagedev/src/../sage startuptime sage/graphs/generators/distance_regular.pyx' [] Traceback (most recent call last): File "/mnt/opt/Sage/sagedev/src/bin/sagestartuptime.py", line 131, in <module> raise ValueError('"' + module_arg + '" does not uniquely determine Sage module.') ValueError: "sage/graphs/generators/distance_regular.pyx" does not uniquely determine Sage module. ERROR: InvocationError for command /bin/sh c '/mnt/opt/Sage/sagedev/src/../sage startuptime sage/graphs/generators/distance_regular.pyx' (exited with code 1) pycodestyle installed: pycodestyle==2.6.0 pycodestyle runtestpre: PYTHONHASHSEED='2313650994' pycodestyle runtest: commands[0]  pycodestyle sage/graphs/generators/distance_regular.pyx unknown option 'description' ignored sage/graphs/generators/distance_regular.pyx:547:28: E701 multiple statements on one line (colon) ERROR: InvocationError for command /mnt/opt/Sage/sagedev/src/.tox/pycodestyle/bin/pycodestyle sage/graphs/generators/distance_regular.pyx (exited with code 1) ___________________________________________________________________________ summary ___________________________________________________________________________ doctest: commands succeeded coverage: commands succeeded ERROR: startuptime: commands failed ERROR: pycodestyle: commands failed
not sure I understand the errors from pycodestyle
and startuptime
 do I have everything set up right?
Is it possible to do only, say, pycodestyle
part?
pycodestyle
output kind of makes sense (except that unknown option 'description' ignored
thing)
does startuptime
only work for modules? (e.g. sage/graphs/
) ?
comment:19 Changed 2 years ago by
comment:20 Changed 2 years ago by
startuptime
unfortunately has an inconsistent interface that wants a module name rather than a filename. Something that should probably be fixed
comment:21 Changed 2 years ago by
can this be mentioned in the dev manual? then it would be a positive review.
comment:22 Changed 2 years ago by
Dependencies:  #30408 → #30408, #30361 

OK, will do, adding to the documentation section from #30361
comment:23 Changed 2 years ago by
Reviewers:  → Dima Pasechnik 

Status:  needs_review → positive_review 
comment:26 Changed 2 years ago by
Branch:  u/mkoeppe/command__sage__tox_ → eba708b89b9935f86d6db8369ef83bcfa1ca9d4b 

Resolution:  → fixed 
Status:  positive_review → closed 
comment:27 Changed 4 weeks ago by
Cc:  chapoton added; chapoton. removed 

Commit:  eba708b89b9935f86d6db8369ef83bcfa1ca9d4b 
New commits:
sage tox