Opened 2 years ago

Closed 2 years ago

Last modified 4 weeks ago

#30410 closed enhancement (fixed)

Command "sage -tox"

Reported by: mkoeppe Owned by:
Priority: major Milestone: sage-9.2
Component: doctest framework Keywords:
Cc: gh-tobiasdiez, 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:

GitHub link to the corresponding issue

Description (last modified by mkoeppe)

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 mkoeppe

Branch: u/mkoeppe/command__sage__tox_

comment:2 Changed 2 years ago by mkoeppe

Authors: Matthias Koeppe
Commit: 5ccccbb0f7a28c878646fcf5c1580cba0e26c475
Description: modified (diff)
Status: newneeds_review

New commits:

5ccccbbsage --tox

comment:3 Changed 2 years ago by gh-tobiasdiez

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)

Last edited 2 years ago by gh-tobiasdiez (previous) (diff)

comment:4 Changed 2 years ago by gh-tobiasdiez

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 in reply to:  3 Changed 2 years ago by mkoeppe

Replying to gh-tobiasdiez:

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.

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 over tox 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 mkoeppe

Dependencies: 30361#30408, #30361

comment:7 Changed 2 years ago by git

Commit: 5ccccbb0f7a28c878646fcf5c1580cba0e26c475857444889f93a3053dda3e195444b3560b35ad62

Branch pushed to git repo; I updated commit sha1. New commits:

1cedc00Add configuration of pycodestyle via tox
dde3b4fsrc/tox.ini: Add testenv:pycodestyle
939e9e0src/tox.ini: Better default arg testenv:pycodestyle
583bde5Merge branch 't/30408/public/build/pycodestyleConfig' into t/30410/command__sage__tox_
8574448src/tox.ini: Add coverage, startuptime; refactor, add descriptions

comment:8 Changed 2 years ago by mkoeppe

Dependencies: #30408, #30361#30408
Description: modified (diff)

I don't think #30361 needs to be a dependency.

comment:9 Changed 2 years ago by gh-tobiasdiez

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 Changed 2 years ago by gh-tobiasdiez

I also think the testing methods in bin/sage should now be removed or deprecated (or at least use tox), or not?

comment:11 in reply to:  9 Changed 2 years ago by mkoeppe

Replying to gh-tobiasdiez:

It should also be possible to add simple forwards in the tox.ini at the root folder

Good idea, will do.

comment:12 in reply to:  10 ; Changed 2 years ago by mkoeppe

Replying to gh-tobiasdiez:

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 in reply to:  12 Changed 2 years ago by mkoeppe

Replying to mkoeppe:

Replying to gh-tobiasdiez:

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 git

Commit: 857444889f93a3053dda3e195444b3560b35ad62eba708b89b9935f86d6db8369ef83bcfa1ca9d4b

Branch pushed to git repo; I updated commit sha1. New commits:

eba708btox.ini: Delegate doctest, coverage, startuptime, pycodestyle to src/tox.ini

comment:15 Changed 2 years ago by mkoeppe

Ready for review

comment:16 Changed 2 years ago by gh-tobiasdiez

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 in reply to:  16 Changed 2 years ago by mkoeppe

Replying to gh-tobiasdiez:

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 Changed 2 years ago by dimpase

I've tried it on a file (with system-wide tox and python):

sage-dev/src $ ../sage -tox  sage/graphs/generators/distance_regular.pyx 
doctest run-test-pre: PYTHONHASHSEED='2313650994'
doctest run-test: commands[0] | sh -c '/mnt/opt/Sage/sage-dev/src/../sage -t -p 0 sage/graphs/generators/distance_regular.pyx'
Running doctests with ID 2020-08-27-17-20-03-d333763b.
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 --warn-long 108.5 --random-seed=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 run-test-pre: PYTHONHASHSEED='2313650994'
coverage run-test: commands[0] | sh -c 'if [ -z "sage/graphs/generators/distance_regular.pyx" ]; then /mnt/opt/Sage/sage-dev/src/../sage --coverageall; else /mnt/opt/Sage/sage-dev/src/../sage --coverage sage/graphs/generators/distance_regular.pyx; fi'
------------------------------------------------------------------------
SCORE sage/graphs/generators/distance_regular.pyx: 100.0% (30 of 30)
------------------------------------------------------------------------
startuptime run-test-pre: PYTHONHASHSEED='2313650994'
startuptime run-test: commands[0] | sh -c '/mnt/opt/Sage/sage-dev/src/../sage --startuptime sage/graphs/generators/distance_regular.pyx'
[]
Traceback (most recent call last):
  File "/mnt/opt/Sage/sage-dev/src/bin/sage-startuptime.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/sage-dev/src/../sage --startuptime sage/graphs/generators/distance_regular.pyx' (exited with code 1)
pycodestyle installed: pycodestyle==2.6.0
pycodestyle run-test-pre: PYTHONHASHSEED='2313650994'
pycodestyle run-test: 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/sage-dev/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/) ?

Last edited 2 years ago by dimpase (previous) (diff)

comment:19 in reply to:  18 Changed 2 years ago by mkoeppe

Replying to dimpase:

Is it possible to do only, say, pycodestyle part?

Yes, tox -e pycodestyle

comment:20 Changed 2 years ago by mkoeppe

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 dimpase

can this be mentioned in the dev manual? then it would be a positive review.

comment:22 Changed 2 years ago by mkoeppe

Dependencies: #30408#30408, #30361

OK, will do, adding to the documentation section from #30361

comment:23 Changed 2 years ago by dimpase

Reviewers: Dima Pasechnik
Status: needs_reviewpositive_review

comment:24 Changed 2 years ago by mkoeppe

Dependencies: #30408, #30361#30408

Thanks!

comment:25 Changed 2 years ago by mkoeppe

Follow-up at #30452 and #30453

comment:26 Changed 2 years ago by vbraun

Branch: u/mkoeppe/command__sage__tox_eba708b89b9935f86d6db8369ef83bcfa1ca9d4b
Resolution: fixed
Status: positive_reviewclosed

comment:27 Changed 4 weeks ago by chapoton

Cc: chapoton added; chapoton. removed
Commit: eba708b89b9935f86d6db8369ef83bcfa1ca9d4b
Note: See TracTickets for help on using tickets.