sagemath/sage

Document "sage -tox"

mkoeppe opened this issue · 42 comments

(from #30410)

As of #30503, sage -advanced shows:

Testing files:

  -t [options] <files|dir> -- test examples in .py, .pyx, .sage
                              or .tex files.  Options:
     --long           -- include lines with the phrase 'long time'
...
  --tox [options] <files|dirs> -- 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 pattern-exclusion 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/sage-9.2#Testing_and_linting_with_tox

See also:

  • #29520 Mention .lgtm.yml in the Developer's Guide
  • #30448 src/tox.ini: Add validation of rst files and docstrings
  • #28936 Meta-ticket: Adopt mainstream Python testing/linting infrastructure, describe in Developer's Guide
  • #30573 src/tox.ini: Add flake8

Depends on #32899

CC: @dimpase @tobiasdiez @fchapoton @jhpalmieri @slel @yuan-zhou

Component: documentation

Author: Matthias Koeppe

Branch/Commit: 4632eb3

Reviewer: Dima Pasechnik, Tobias Diez

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

comment:1

Perhaps we also need sage -btox and sage -btoxp (in analogy to sage -bt, sage -btp)

Changed dependencies from #30410, to #30410, #30452

Changed dependencies from #30410, #30452 to #30410, #30452, #30467

Changed dependencies from #30410, #30452, #30467 to #30361, #30410, #30452, #30467

Changed dependencies from #30361, #30410, #30452, #30467 to #30361, #30410, #30452, #30467, #30503

Description changed:

--- 
+++ 
@@ -1,2 +1,36 @@
 (from #30410)
 
+As of #30503, `sage -advanced` shows:
+
+```
+Testing files:
+
+  -t [options] <files|dir> -- test examples in .py, .pyx, .sage
+                              or .tex files.  Options:
+     --long           -- include lines with the phrase 'long time'
+...
+  --tox [options] <files|dirs> -- 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 pattern-exclusion 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.
+
+
+
+
+
+

Description changed:

--- 
+++ 
@@ -31,6 +31,7 @@
 
 
 
+See also:
+- #29520 Mention `.lgtm.yml` in the Developer's Guide
 
 
-

Description changed:

--- 
+++ 
@@ -33,5 +33,6 @@
 
 See also:
 - #29520 Mention `.lgtm.yml` in the Developer's Guide
+- #30448 src/tox.ini: Add validation of rst files and docstrings
+- #28936 Meta-ticket: Adopt mainstream Python testing/linting infrastructure, describe in Developer's Guide
 
-

Changed dependencies from #30361, #30410, #30452, #30467, #30503 to #30361, #30410, #30452, #30467, #30503, #30416

Changed dependencies from #30361, #30410, #30452, #30467, #30503, #30416 to #30361, #30410, #30452, #30467, #30503, #30544

Description changed:

--- 
+++ 
@@ -33,6 +33,7 @@
 
 See also:
 - #29520 Mention `.lgtm.yml` in the Developer's Guide
-- #30448 src/tox.ini: Add validation of rst files and docstrings
+- #30448 `src/tox.ini`: Add validation of rst files and docstrings
 - #28936 Meta-ticket: Adopt mainstream Python testing/linting infrastructure, describe in Developer's Guide
+- #30573 `src/tox.ini`: Add flake8
 

Description changed:

--- 
+++ 
@@ -27,7 +27,7 @@
 
 ```
 
-In this ticket, we expand the section in the developers' guide added in #30361 to cover these tools.
+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/sage-9.2#Testing_and_linting_with_tox
 
 
 
comment:13

Sage development has entered the release candidate phase for 9.3. Setting a new milestone for this ticket based on a cursory review.

Changed dependencies from #30361, #30410, #30452, #30467, #30503, #30544 to none

comment:16

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

make is sage-the-distribution-specific.

sage -t and sage --tox work in all settings.

That's why we should change it.

comment:18

why break something that works, and is superior in usability?

comment:19

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

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

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

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 (sage-the-distribution-specific) that make test offers over sage -t.

Dependencies: #32899

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

7d31d58src/doc/en/developer/tools.rst: Add codespell
comment:27

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 variable mone which codespell thinks is misspelled?

I've added a bit of documentation for codespell

Author: Matthias Koeppe

slel commented
comment:29

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/build-system/pyproject-toml/>`_
+packages via the file
+`pyproject.toml <https://pip.pypa.io/en/stable/reference/build-system/pyproject-toml/>`_
-such optional dependencies as `extras_require <https://setuptools.pypa.io/en/latest/userguide/dependency_management.html#optional-dependencies>`_ in ``setup.cfg``
+such optional dependencies as
+`extras_require <https://setuptools.pypa.io/en/latest/userguide/dependency_management.html#optional-dependencies>`_
+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

Feel free to push changes of this kind to the ticket

comment:31

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 non-standard 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.

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

c3f566bsrc/doc/en/developer/tools.rst: Document tox -e doctest,coverage,startuptime and explain in which venv they runs
426ca9esrc/doc/en/developer/tools.rst: Add guidance for developers regarding pycodestyle[-minimal]

Changed commit from 7d31d58 to 426ca9e

comment:33

Thanks for the comments! I have made some changes

comment:34

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

Changed commit from 426ca9e to 4632eb3

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

4632eb3src/doc/en/developer/tools.rst: Add section on relint
comment:36

Replying to @mkoeppe:

Thanks for the comments! I have made some changes

These changes look good to me, thanks!

comment:37

If there are no more comments, let's get this in please

comment:38

OK

Reviewer: Dima Pasechnik, Tobias Diez

comment:39

Thanks!