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: sage-9.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:

Status badges

Description (last modified by Matthias Köppe)

(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

Change History (41)

comment:1 Changed 2 years ago by Matthias Köppe

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

comment:2 Changed 2 years ago by Matthias Köppe

Dependencies: #30410,#30410, #30452

comment:3 Changed 2 years ago by Matthias Köppe

Dependencies: #30410, #30452#30410, #30452, #30467

comment:4 Changed 2 years ago by Matthias Köppe

Dependencies: #30410, #30452, #30467#30361, #30410, #30452, #30467

comment:5 Changed 2 years ago by Matthias Köppe

Cc: Samuel Lelièvre added
Dependencies: #30361, #30410, #30452, #30467#30361, #30410, #30452, #30467, #30503

comment:6 Changed 2 years ago by Matthias Köppe

Description: modified (diff)

comment:7 Changed 2 years ago by Matthias Köppe

Description: modified (diff)

comment:8 Changed 2 years ago by Matthias Köppe

Dependencies: #30361, #30410, #30452, #30467, #30503#30361, #30410, #30452, #30467, #30503, #30416
Description: modified (diff)

comment:9 Changed 2 years ago by Matthias Köppe

Dependencies: #30361, #30410, #30452, #30467, #30503, #30416#30361, #30410, #30452, #30467, #30503, #30544

comment:10 Changed 2 years ago by Matthias Köppe

Description: modified (diff)

comment:11 Changed 2 years ago by Matthias Köppe

Milestone: sage-9.2sage-9.3

comment:12 Changed 23 months ago by Matthias Köppe

Cc: Yuan Zhou added
Description: modified (diff)

comment:13 Changed 20 months ago by Matthias Köppe

Milestone: sage-9.3sage-9.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 Matthias Köppe

Milestone: sage-9.4sage-9.5

comment:15 Changed 13 months ago by Matthias Köppe

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 Dima Pasechnik

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 Matthias Köppe

make is sage-the-distribution-specific.

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

That's why we should change it.

comment:18 Changed 13 months ago by Dima Pasechnik

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

comment:19 Changed 13 months ago by Matthias Köppe

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 Dima Pasechnik

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 Changed 13 months ago by John Palmieri

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 Matthias Köppe

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.

comment:23 Changed 13 months ago by Matthias Köppe

Summary: Document "sage -tox", stop advertising "make ...test..."Document "sage -tox"

comment:24 Changed 13 months ago by Matthias Köppe

Dependencies: #32899

comment:25 Changed 13 months ago by Matthias Köppe

Branch: u/mkoeppe/document__sage__tox_

comment:26 Changed 13 months ago by git

Commit: 7d31d586f330ac9175c94cad0cf6422928c12b5f

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

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

comment:27 in reply to:  21 Changed 13 months ago by Matthias Köppe

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

comment:28 Changed 13 months ago by Matthias Köppe

Authors: Matthias Koeppe
Status: newneeds_review

comment:29 Changed 13 months ago by Samuel Lelièvre

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 Changed 13 months ago by Matthias Köppe

Feel free to push changes of this kind to the ticket

comment:31 Changed 13 months ago by Tobias Diez

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.

comment:32 Changed 13 months ago by git

Commit: 7d31d586f330ac9175c94cad0cf6422928c12b5f426ca9e0f8612bc7b5c707c9a9f4c0c65fe43a93

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]

comment:33 Changed 13 months ago by Matthias Köppe

Thanks for the comments! I have made some changes

comment:34 in reply to:  30 Changed 13 months ago by Matthias Köppe

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 git

Commit: 426ca9e0f8612bc7b5c707c9a9f4c0c65fe43a934632eb32bb3f8af20bc63ac2baf0241abeca9269

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

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

comment:36 in reply to:  33 Changed 12 months ago by Tobias Diez

Replying to mkoeppe:

Thanks for the comments! I have made some changes

These changes look good to me, thanks!

comment:37 Changed 12 months ago by Matthias Köppe

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

comment:38 Changed 12 months ago by Dima Pasechnik

Reviewers: Dima Pasechnik, Tobias Diez
Status: needs_reviewpositive_review

OK

comment:39 Changed 12 months ago by Matthias Köppe

Thanks!

comment:40 Changed 12 months ago by Matthias Köppe

Priority: majorcritical

comment:41 Changed 12 months ago by Volker Braun

Branch: u/mkoeppe/document__sage__tox_4632eb32bb3f8af20bc63ac2baf0241abeca9269
Resolution: fixed
Status: positive_reviewclosed
Note: See TracTickets for help on using tickets.