Command "sage -tox" #30410

mkoeppe · 2020-08-20T21:14:11Z

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).

Depends on #30408

CC: @tobiasdiez @dimpase @jhpalmieri @fchapoton

Component: doctest framework

Author: Matthias Koeppe

Branch: eba708b

Reviewer: Dima Pasechnik

Issue created by migration from https://trac.sagemath.org/ticket/30410

The text was updated successfully, but these errors were encountered:

mkoeppe · 2020-08-20T22:41:05Z

Branch: u/mkoeppe/command__sage__tox_

mkoeppe · 2020-08-20T22:41:57Z

New commits:

`5ccccbb`	`sage --tox`

mkoeppe · 2020-08-20T22:41:57Z

Author: Matthias Koeppe

mkoeppe · 2020-08-20T22:41:57Z

Commit: 5ccccbb

tobiasdiez · 2020-08-21T09:25:33Z

comment:3

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)

tobiasdiez · 2020-08-21T10:39:43Z

comment:4

Whatever the final interface looks like, the documentation needs to be changed as well to reflect this. In particular, this ticket depends on #30361.

tobiasdiez · 2020-08-21T10:39:43Z

Dependencies: 30361

mkoeppe · 2020-08-21T16:25:50Z

comment:5

Replying to @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.

mkoeppe · 2020-08-21T16:36:18Z

Changed dependencies from 30361 to #30408, #30361

sagetrac-git · 2020-08-21T17:27:57Z

Changed commit from 5ccccbb to 8574448

sagetrac-git · 2020-08-21T17:27:57Z

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`

mkoeppe · 2020-08-21T17:31:53Z

Changed dependencies from #30408, #30361 to #30408

mkoeppe · 2020-08-21T17:31:53Z

comment:8

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

tobiasdiez · 2020-08-21T17:37:30Z

comment:9

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.

tobiasdiez · 2020-08-21T17:40:54Z

comment:10

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

mkoeppe · 2020-08-21T17:41:06Z

comment:11

Replying to @tobiasdiez:

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

Good idea, will do.

mkoeppe · 2020-08-21T17:41:48Z

comment:12

Replying to @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...

mkoeppe · 2020-08-21T17:43:25Z

comment:13

Replying to @mkoeppe:

Replying to @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.

sagetrac-git · 2020-08-21T22:41:54Z

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

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

sagetrac-git · 2020-08-21T22:41:54Z

Changed commit from 8574448 to eba708b

mkoeppe · 2020-08-27T04:52:02Z

comment:15

Ready for review

tobiasdiez · 2020-08-27T12:47:20Z

comment:16

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.

mkoeppe · 2020-08-27T16:22:25Z

comment:17

Replying to @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.

dimpase · 2020-08-27T17:58:20Z

comment:18

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/) ?

mkoeppe · 2020-08-27T17:59:49Z

comment:19

Replying to @dimpase:

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

Yes, tox -e pycodestyle

mkoeppe · 2020-08-27T18:00:58Z

comment:20

startuptime unfortunately has an inconsistent interface that wants a module name rather than a filename. Something that should probably be fixed

dimpase · 2020-08-27T19:21:34Z

comment:21

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

mkoeppe · 2020-08-27T20:25:54Z

comment:22

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

mkoeppe · 2020-08-27T20:25:54Z

Changed dependencies from #30408 to #30408, #30361

dimpase · 2020-08-27T20:58:37Z

Reviewer: Dima Pasechnik

mkoeppe · 2020-08-27T21:04:01Z

Changed dependencies from #30408, #30361 to #30408

mkoeppe · 2020-08-27T21:04:01Z

comment:24

Thanks!

mkoeppe · 2020-08-27T21:04:47Z

comment:25

Follow-up at #30452 and #30453

vbraun · 2020-09-06T21:51:36Z

Changed branch from u/mkoeppe/command__sage__tox_ to eba708b

fchapoton · 2023-01-08T08:57:31Z

Changed commit from eba708b to none

mkoeppe added this to the sage-9.2 milestone Aug 20, 2020

mkoeppe added c: doctest framework labels Aug 20, 2020

This comment has been minimized.

Sign in to view

mkoeppe added the s: needs review label Aug 20, 2020

This comment has been minimized.

Sign in to view

dimpase added s: positive review and removed s: needs review labels Aug 27, 2020

vbraun removed the s: positive review label Sep 6, 2020

vbraun closed this as completed in dfa0d22 Sep 6, 2020

Command "sage -tox" #30410

Command "sage -tox" #30410

Comments

mkoeppe commented Aug 20, 2020

mkoeppe commented Aug 20, 2020

mkoeppe commented Aug 20, 2020

mkoeppe commented Aug 20, 2020

mkoeppe commented Aug 20, 2020

This comment has been minimized.

tobiasdiez commented Aug 21, 2020

tobiasdiez commented Aug 21, 2020

tobiasdiez commented Aug 21, 2020

mkoeppe commented Aug 21, 2020

mkoeppe commented Aug 21, 2020

sagetrac-git mannequin commented Aug 21, 2020

sagetrac-git mannequin commented Aug 21, 2020

mkoeppe commented Aug 21, 2020

This comment has been minimized.

mkoeppe commented Aug 21, 2020

tobiasdiez commented Aug 21, 2020

tobiasdiez commented Aug 21, 2020

mkoeppe commented Aug 21, 2020

mkoeppe commented Aug 21, 2020

mkoeppe commented Aug 21, 2020

sagetrac-git mannequin commented Aug 21, 2020

sagetrac-git mannequin commented Aug 21, 2020

mkoeppe commented Aug 27, 2020

tobiasdiez commented Aug 27, 2020

mkoeppe commented Aug 27, 2020

dimpase commented Aug 27, 2020

mkoeppe commented Aug 27, 2020

mkoeppe commented Aug 27, 2020

dimpase commented Aug 27, 2020

mkoeppe commented Aug 27, 2020

mkoeppe commented Aug 27, 2020

dimpase commented Aug 27, 2020

mkoeppe commented Aug 27, 2020

mkoeppe commented Aug 27, 2020

mkoeppe commented Aug 27, 2020

vbraun commented Sep 6, 2020

fchapoton commented Jan 8, 2023