Opened 2 years ago
Closed 12 months ago
#30453 closed enhancement (fixed)
Document "sage tox"
Reported by:  Matthias Köppe  Owned by:  

Priority:  critical  Milestone:  sage9.5 
Component:  documentation  Keywords:  
Cc:  Dima Pasechnik, Tobias Diez, Frédéric Chapoton, John Palmieri, Samuel Lelièvre, Yuan Zhou  Merged in:  
Authors:  Matthias Koeppe  Reviewers:  Dima Pasechnik, Tobias Diez 
Report Upstream:  N/A  Work issues:  
Branch:  4632eb3 (Commits, GitHub, GitLab)  Commit:  4632eb32bb3f8af20bc63ac2baf0241abeca9269 
Dependencies:  #32899  Stopgaps: 
Description (last modified by )
(from #30410)
As of #30503, sage advanced
shows:
Testing files: t [options] <filesdir>  test examples in .py, .pyx, .sage or .tex files. Options: long  include lines with the phrase 'long time' ... tox [options] <filesdirs>  general entry point for testing and linting of the Sage library e <envlist>  run specific test environments (default: run all) doctest  run the Sage doctester (same as "sage t") coverage  give information about doctest coverage of files (same as "sage coverage[all]") startuptime  display how long each component of Sage takes to start up (same as "sage startuptime") pycodestyle  check against the Python style conventions of PEP8 relint  check whether some forbidden patterns appear (includes all patchbot patternexclusion plugins) codespell  check for misspelled words in source code p auto  run test environments in parallel help  show tox help
In this ticket, we expand the section in the developers' guide added in #30361 to cover these tools, expanding upon https://wiki.sagemath.org/ReleaseTours/sage9.2#Testing_and_linting_with_tox
See also:
Change History (41)
comment:1 Changed 2 years ago by
comment:2 Changed 2 years ago by
Dependencies:  #30410, → #30410, #30452 

comment:3 Changed 2 years ago by
Dependencies:  #30410, #30452 → #30410, #30452, #30467 

comment:4 Changed 2 years ago by
Dependencies:  #30410, #30452, #30467 → #30361, #30410, #30452, #30467 

comment:5 Changed 2 years ago by
Cc:  Samuel Lelièvre added 

Dependencies:  #30361, #30410, #30452, #30467 → #30361, #30410, #30452, #30467, #30503 
comment:6 Changed 2 years ago by
Description:  modified (diff) 

comment:7 Changed 2 years ago by
Description:  modified (diff) 

comment:8 Changed 2 years ago by
Dependencies:  #30361, #30410, #30452, #30467, #30503 → #30361, #30410, #30452, #30467, #30503, #30416 

Description:  modified (diff) 
comment:9 Changed 2 years ago by
Dependencies:  #30361, #30410, #30452, #30467, #30503, #30416 → #30361, #30410, #30452, #30467, #30503, #30544 

comment:10 Changed 2 years ago by
Description:  modified (diff) 

comment:11 Changed 2 years ago by
Milestone:  sage9.2 → sage9.3 

comment:12 Changed 23 months ago by
Cc:  Yuan Zhou added 

Description:  modified (diff) 
comment:13 Changed 20 months ago by
Milestone:  sage9.3 → sage9.4 

Sage development has entered the release candidate phase for 9.3. Setting a new milestone for this ticket based on a cursory review.
comment:14 Changed 17 months ago by
Milestone:  sage9.4 → sage9.5 

comment:15 Changed 13 months ago by
Dependencies:  #30361, #30410, #30452, #30467, #30503, #30544 

Summary:  Document "sage tox" → Document "sage tox", stop advertising "make ...test..." 
comment:16 Changed 13 months ago by
I don't think we should stop advertising "make ...test...". Intefaces to tox, pytest, etc are insane, one needs to look in the manuals all the time.
comment:17 Changed 13 months ago by
make
is sagethedistributionspecific.
sage t
and sage tox
work in all settings.
That's why we should change it.
comment:19 Changed 13 months ago by
Everyone already has to learn about sage t
for testing, because that's the only way that you can test something specific.
So advertising both sage t
and make test
is one too many.
comment:20 Changed 13 months ago by
my brain is small, and while I know that ./sage tp log all >logs/ptest.log
is more or less the same (I would still need to look up the correct way to pipe stderr) as make ptest
 I wish I knew something more useful instead.
this is really akin to "who needs makefiles, you can just type the compiler calls by hand" nonsense.
comment:21 followup: 27 Changed 13 months ago by
Something like make test
or make check
is common with various packages, and I don't see a reason to stop advertising it. Three options doesn't seem like too many, since they do different things (./sage t ...
tests specific files, make ...test...
tests all files and logs the output, ./sage tox ...
tests in a different way). Maybe part of my resistance is that I don't understand how to use ./sage tox ...
effectively; with make ptestlong
, I can open up the log file while it's going and see the progress, but I haven't figured out how to do that with tox
. When I just ran sage tox src/sage/homology
, it ended in an error, but I'm not sure why. Because the code uses a variable mone
which codespell
thinks is misspelled?
comment:22 Changed 13 months ago by
Note that I was suggesting to replace make test
in the manuals by sage t
, not by sage tox
.
Note that all make *test*
targets have equivalents in sage t all
with additional options, so I wouldn't say that the difference is that ./sage t ...
tests specific files and make ...test...
tests all files.
But you have a good point about logging. That is indeed something (sagethedistributionspecific) that make test
offers over sage t
.
comment:23 Changed 13 months ago by
Summary:  Document "sage tox", stop advertising "make ...test..." → Document "sage tox" 

comment:24 Changed 13 months ago by
Dependencies:  → #32899 

comment:25 Changed 13 months ago by
Branch:  → u/mkoeppe/document__sage__tox_ 

comment:26 Changed 13 months ago by
Commit:  → 7d31d586f330ac9175c94cad0cf6422928c12b5f 

Branch pushed to git repo; I updated commit sha1. New commits:
7d31d58  src/doc/en/developer/tools.rst: Add codespell

comment:27 Changed 13 months ago by
Replying to jhpalmieri:
When I just ran
sage tox src/sage/homology
, it ended in an error, but I'm not sure why. Because the code uses a variablemone
whichcodespell
thinks is misspelled?
I've added a bit of documentation for codespell
comment:28 Changed 13 months ago by
Authors:  → Matthias Koeppe 

Status:  new → needs_review 
comment:29 Changed 13 months ago by
Many shell code blocks are indented by 2 spaces, aren't 4 required by rst / Sphinx?
Typo (a > an):
packages, that prefix needs to be a implicit namespace package, i.e., there +packages, that prefix needs to be an implicit namespace package, i.e., there
Perhaps split some long lines:
packages via the file `pyproject.toml <https://pip.pypa.io/en/stable/reference/buildsystem/pyprojecttoml/>`_ +packages via the file +`pyproject.toml <https://pip.pypa.io/en/stable/reference/buildsystem/pyprojecttoml/>`_
such optional dependencies as `extras_require <https://setuptools.pypa.io/en/latest/userguide/dependency_management.html#optionaldependencies>`_ in ``setup.cfg`` +such optional dependencies as +`extras_require <https://setuptools.pypa.io/en/latest/userguide/dependency_management.html#optionaldependencies>`_ +in ``setup.cfg``
 VS Code: Activate by adding the setting ``"python.linting.pycodestyleEnabled": true``, see `official VS Code documentation <https://code.visualstudio.com/docs/python/linting>`__ for details. + VS Code: Activate by adding the setting + ``"python.linting.pycodestyleEnabled": true``, see + `official VS Code documentation <https://code.visualstudio.com/docs/python/linting>`__ + for details.
*Note*: Currently, only the package :mod:`sage.manifolds` is checked. Further packages can be added in the ``pyrightconfig.json`` file. +*Note*: Currently, only the package :mod:`sage.manifolds` is checked. +Further packages can be added in the ``pyrightconfig.json`` file.
comment:30 followup: 34 Changed 13 months ago by
Feel free to push changes of this kind to the ticket
comment:31 Changed 13 months ago by
What I'm missing (as it confused me quite a bit) is a remark about the python (and other packages) used by tox and how/if a new virtual environment is created, especially for src/tox.ini
. "Normally", you have environments like
envlist = py{27,34,36}django{15,16}{sqlite,mysql}, docs
That is, you run your test command in different configurations and new virtual environments; with additional special commands like documentation or linting. Sage's usage of tox is rather nonstandard in this regard, so I think its good to point this out.
Concerning the documentation of pycodestyle, I'm not sure if the output of the various tox commands adds much. Instead, maybe stress that minimal
needs to pass for a ticket to be merged, while the other configuration is what sage is striving for in the future, so progress in that direction is welcome.
comment:32 Changed 13 months ago by
Commit:  7d31d586f330ac9175c94cad0cf6422928c12b5f → 426ca9e0f8612bc7b5c707c9a9f4c0c65fe43a93 

comment:34 Changed 13 months ago by
Replying to mkoeppe:
Feel free to push changes of this kind to the ticket
... or post the unedited output of git diff
so I can apply these changes
comment:35 Changed 12 months ago by
Commit:  426ca9e0f8612bc7b5c707c9a9f4c0c65fe43a93 → 4632eb32bb3f8af20bc63ac2baf0241abeca9269 

Branch pushed to git repo; I updated commit sha1. New commits:
4632eb3  src/doc/en/developer/tools.rst: Add section on relint

comment:36 Changed 12 months ago by
Replying to mkoeppe:
Thanks for the comments! I have made some changes
These changes look good to me, thanks!
comment:38 Changed 12 months ago by
Reviewers:  → Dima Pasechnik, Tobias Diez 

Status:  needs_review → positive_review 
OK
comment:40 Changed 12 months ago by
Priority:  major → critical 

comment:41 Changed 12 months ago by
Branch:  u/mkoeppe/document__sage__tox_ → 4632eb32bb3f8af20bc63ac2baf0241abeca9269 

Resolution:  → fixed 
Status:  positive_review → closed 
Perhaps we also need
sage btox
andsage btoxp
(in analogy tosage bt
,sage btp
)