add nox config; update CI

[2bndy5]

also ran pre-commit hooks via nox.

resolves #223

[2bndy5]

It also occurred to me that we could include the docs' build in the code coverage, but I didn't pursue that because it isn't as definitive as unit tests.

[2bndy5]

On a side note, I would've liked the coverage data to be combined from each OS-specific job, but the concurrency of the matrix in CI would definitely be a problem. So, I just have the CI create a coverage report for each OS.

[jbms]

It also occurred to me that we could include the docs' build in the code coverage, but I didn't pursue that because it isn't as definitive as unit tests.

Yeah, in my experience coverage metrics are most useful when used carefully, e.g. measuring the coverage from just a specific set of unit tests known to actually validate the code's behavior. It is easy to achieve high coverage by just executing the code without properly validating the behavior.

[2bndy5]

It is true the docs aren't a great source of truth for code coverage. I can remove them from the tests job and put that back into the build job.

There is a way to get context in the comprehensive coverage (HTML) report: But I see now that I forgot to include the config in the toml to enable this; it will be added shortly.

My thinking is that we don't employ a fool proof or intuitive/robust way to "properly validate" the generated docs' output (especially considering the builder used). Some of the tests just verify that the test's build went without incident; that recent test_system_font() is an example of this because I had to visually inspect the rendered HTML to verify the expected font was used. That is why I figured the docs build may apply to code coverage.

---

On a somewhat related note, I'd like to use a env var to conditionally add the repo's root folder to the sys.path in conf.py, so I don't have to keep installing the theme when developing a solution. FYI, I've been doing this locally for quite some time now:

--- a/docs/conf.py
+++ b/docs/conf.py
@@ -14,6 +14,7 @@ import typing
 from typing_extensions import Literal

 sys.path.insert(0, os.path.abspath("."))
+sys.path.insert(0, os.path.abspath(".."))

 import docutils
 import sphinx

[2bndy5]

Docs are now built after installing the wheel (as was done before).

I know that the build and test jobs could be combined, but using the concurrency of 2 separate jobs makes the CI finish faster. And, we don't need to install all supported versions of python to build the pkg and docs, so it still makes sense to run the unit tests separately.

[jbms]

On a somewhat related note, I'd like to use a env var to conditionally add the repo's root folder to the sys.path in conf.py, so I don't have to keep installing the theme when developing a solution.

I normally use pip install -e . --- that installs the package in development mode, where the "installed version" just points back to the source directory, so you can modify the source files without having to reinstall again.

[2bndy5]

I can do that since it doesn't seem to go through the long steps of copying everything... I'll remove the env var stuff from the conf.py and noxfile.py

[jbms]

This looks good to me --- the one thing you might consider is splitting up some of the configurations over multiple runs, especially on macos and windows where` it is taking 13-15 minutes.

[2bndy5]

How so? You mean only certain sphinx versions tested on the macos and windows runners?

[jbms]

How so? You mean only certain sphinx versions tested on the macos and windows runners?

Previously we had a separate job for each python version and each platform. That is probably excessive but we could split the macos and windows configurations across 2 or 3 different jobs, e.g. by specifying as part of the matrix configuration a parameter that will be passed to nox to restrict which configurations are tested.

[2bndy5]

I can adjust the tests matrix to use different nox sessions...

test:
  strategy:
    matrix:
      include:
        - os: 'ubuntu-latest'
          nox-sessions: 'tests' # test all sphinx versions on all python versions
        - os: 'windows-latest'
          nox-sessions: 'tests-3.11'  # only test all sphinx versions on python v3.11
        - os: 'macos-latest'
          nox-sessions: 'tests-3.11(sphinx6)'  # only test sphinx v6 on python v3.11
  steps:
    run: nox -s ${{ matrix.nox-sessions }}

[jbms]

Yeah exactly like that except that we can have multiple windows nacos entries in the matrix so that we still test all configurations, just in parallel rather than sequentially

[2bndy5]

Ah. It will fail if a certain python version is not installed (or added to PATH) -- this behavior is the default when the env var CI is asserted. This will require an extra param to nox to allow skipping a python version that is not present in PATH.

I'm currently doing it sequentially because there's no reliable way to cache (or save as artifacts) the multiple coverage data files across concurrent jobs and then combine them when all tests are finished.

[2bndy5]

I can cache each set of coverage data files and then use a second job to combine and report the results. But this would be very long CI script since each concurrent job's cache will have to be restored individually (actions/cache keys doesn't support globs). I thought about using artifacts and downloading all artifacts after the tests are done, but this would include downloading all built docs and pkg artifacts.

[jbms]

Artifact upload/download would definitely be a better fit than the cache. You can choose to download just a particular named artifact -- you don't have to download everything.

[2bndy5]

You can choose to download just a particular named artifact -- you don't have to download everything.

Not if we're creating a single artifact concurrently. I tried this using 1 artifact with a single name, but the CI will fail if an artifact is currently being written to by another concurrent job.

[2bndy5]

Similar to actions/cache, downloading artifacts by name doesn't support glob patterns.

[jbms]

Yeah I imagined you would only download the artifacts from a separate job that runs after all the other jobs complete, and combine the coverage data.

[2bndy5]

Yeah. Doing that will require separate artifacts per concurrent job, and the final reporting job will have to download all artifacts regardless of name (just to get the coverage data artifacts).

[2bndy5]

I'll submit the change and see if it desirable...

[2bndy5]

ok, I got the coverage report to combine from artifacts of concurrently run tests. I also applied concurrency to the check_conventions job.

Now we should discuss which jobs to exclude from the test matrix. I was thinking that

• macos should run tests only on latest py and latest sphinx versions because it is the slowest runner there is.
• windows should run tests either
  1. on all sphinx versions but only on latest py version
  2. on all py versions but only only on latest sphinx version
• ubuntu will still run tests on all supported sphinx and py versions because it is the champion of runners' speed.

The biggest detriment in speed for non-ubuntu runners is the "install graphviz" step, so...

[2bndy5]

I'm just remembering that py3.11 release notes boasted "improved performance on Windows", so I'll just use latest py version with all sphinx versions on windows runner.

[2bndy5]

While I'm still at it... Did you want to limit the sphinx builders used for the docs in macOS or Windows?

[jbms]

To be clear, I wasn't suggesting skipping any variants, just having multiple windows and macos jobs in the matrix and distributing the variants among them.

[2bndy5]

oh! I can revert the limitations.

[jbms]

The current configuration is maybe more parallelized than necessary, but it is okay.

What I had in mind was that for ubuntu, there would just be a single test job. For macos and windows, we might have one test job (i.e. matrix include entry) that sequentially tests on python 3.8 and 3.9 (all sphinx versions), and another test job (i.e. matrix include entry) that sequentially tests on python 3.10 and 3.11.

[2bndy5]

Oh. Well you're welcome to tweak it further. I'm kinda an all-or-nothing guy. The logs are somewhat clearer when it is all parallelized.

[2bndy5]

I just learned about just. It seems like a feasible alternative to nox (and makefiles) because a python venv is not mandated (and the shell used is configurable). The caching of a python venv in CI would be slightly less implicit (venv names would have to be defined in the justfile).

Using @nox.session(python=False) installs things to the system python env. Lately I've been avoiding the use of nox locally to prevent nox -s docs from polluting my system python env. just seems like a viable approach, but it isn't installed via pip (see using a pkg-mgr or using a gh action).