Allow tests that are specific to Sage's build system to be skipped

[embray]

Adds an 'optional' tag called 'build' (name to be bikeshedded if necessary) specifically for tests related to sage-the-distribution and Sage's build system.

This allows downstream distributions that package Sage differently to skip these tests, but they are enabled by default when the tests are run from a build in Sage's source repository.

This provides a possible solution to #27621.

[embray]

Added # optional - build to several tests in sage.misc.package, but perhaps some of the downstream packagers could mention some other problem test if there are any.

---

New commits:

​99699e2	Trac #27635: Add the --optional=build tag by default when run out of the Sage source repository.
​c424acf	fixup
​d4ba444	Trac #27635: Add 'optional - build' to several sage-the-distribution related tests.

[git]

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

​d96a081	Trac #27635: Add the --optional=build tag by default when run out of the Sage source repository.
​76fae85	Trac #27635: Add 'optional - build' to several sage-the-distribution related tests.

[gh-timokau]

Thank you! The only possible issue I can see with this (and the reason why I still create empty dummy files in installed) is that people may use is_package_installed in scripts to gate certain features.

They really *should* be using the features feature ( :) ) for that though, so maybe that's okay. Definitively net positive overall.

[arojas]

Other tests that should have this flag:

• The os.path.isfile(started_file) test in sage/all.py:288
• Tests that involve sage --root and sage --info in tests/cmdline.py:198-234

[embray]

Replying to gh-timokau:

Thank you! The only possible issue I can see with this (and the reason why I still create empty dummy files in installed) is that people may use is_package_installed in scripts to gate certain features.

That's interesting... One thing that is definitely on my todo list, though I have no idea when I'll get to it, is to have an easy way for distros to plug into sage's configure script, so that it can use the distro's packaging system to check for the required dependencies, and possibly also output these files (although they are of less interest in that case).

They really *should* be using the features feature ( :) ) for that though, so maybe that's okay. Definitively net positive overall.

Yep!

[embray]

Replying to arojas:

Other tests that should have this flag:

• The os.path.isfile(started_file) test in sage/all.py:288
• Tests that involve sage --root and sage --info in tests/cmdline.py:198-234

Thank you for pointing them out. I'll add them.

[jhpalmieri]

Right now the doctesting flags are set both in the main Makefile and in sage-runtests. Should the same happen here? At #27124, @gh-timokau suggested not even trying to guess whether to use the build flag but just make it the default, for example via the existing variable TESTALL_FLAGS. Then people can modify that variable as they like, perhaps as they already do to omit dochtml.

The build flag should probably also be documented in the parser help in sage-runtests: if "build" is listed, will also run the tests relying on Sage's packaging system or something like that.

[fbissey]

Replying to embray:

Replying to gh-timokau:

Thank you! The only possible issue I can see with this (and the reason why I still create empty dummy files in installed) is that people may use is_package_installed in scripts to gate certain features.

That's interesting... One thing that is definitely on my todo list, though I have no idea when I'll get to it, is to have an easy way for distros to plug into sage's configure script, so that it can use the distro's packaging system to check for the required dependencies, and possibly also output these files (although they are of less interest in that case).

Similarly I have is_package_installed returning False in sage-on-gentoo so as not to remove it everywhere. That used to be a big patch. I only remove it from code where Gentoo can offer a replacement.

They really *should* be using the features feature ( :) ) for that though, so maybe that's okay. Definitively net positive overall.

Yep!

Yes at runtime definitely.

At configure time it is still used for optional bindings that have code. At the moment I replace it by querying environment variables in sage_setup/optional_extension.py. If it could be replaced by a setup.cfg that could be generated by configure - or the package building system - that would be great. On the other hand that would mean people couldn't just do a sage -i .... followed by sage -b to rebuild for the package they have just added.

Lastly the doctesting system relies on is_package_installed to figure some of the options you have enabled when passed --optional=all. I just cannot think of how to replace that.

[git]

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

​a015888

Trac #27635: Skip more tests that may not make sense outside a Sage build from

[embray]

Replying to jhpalmieri:

Right now the doctesting flags are set both in the main Makefile and in sage-runtests. Should the same happen here? At #27124, @gh-timokau suggested not even trying to guess whether to use the build flag but just make it the default, for example via the existing variable TESTALL_FLAGS. Then people can modify that variable as they like, perhaps as they already do to omit dochtml.

I think I see what you're saying. I'll add it to TESTALL_FLAGS: It would have to be since otherwise it is only appended if running sage -t directly without explicitly providing an --optional flag. I think the default guessing behavior is pretty good as-is though unless someone can find a strong counter-example.

The build flag should probably also be documented in the parser help in sage-runtests: if "build" is listed, will also run the tests relying on Sage's packaging system or something like that.

Done.

[git]

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

​a282e09	Trac #27635: Add 'build' to the default TESTALL_FLAGS in the main Makefile
​2533f32	Trac #27635: Document the 'build' option flag in the documentation for sage-runtests

[jdemeyer]

What about replacing

    if (SAGE_ROOT and os.path.isdir(SAGE_ROOT) and
            os.path.exists(os.path.join(SAGE_ROOT, '.git')) and
            os.path.isfile(os.path.join(SAGE_ROOT, 'sage'))):

by

    if (SAGE_ROOT and os.path.isdir(os.path.join(SAGE_ROOT, 'build'))):

This way, we don't check for .git which we don't actually need. Second, you do need the build directory for the build tests to succeed (and this happens to align with the name --optional=build). Third, the check for os.path.isdir(SAGE_ROOT) is superfluous: if a file inside that directory exists, then surely the directory itself exists.

[gh-timokau]

I agree, there is no reason a distro build can't happen from a git checkout.

How about marking the python safe directory test (src/sage/doctest/control.py) as build? Its not strictly about the sage build system, but it is specific to the way sage builds its python. I still don't think that should be tested at all, but that way at least packagers don't have to patch it out anymore.

[jhpalmieri]

There is nothing to stop anyone from opening up a new ticket and marking more tests as build. Let's test this ticket thoroughly and make sure it works; that's more important than marking every one of them here.

[fbissey]

I abstained from asking for more stuff to be added myself because I thought build would become a misnomer. If we want to extend we can re-name and add more stuff in a follow up - once we are sure the concept is sound. I certainly have my own laundry list.

[jhpalmieri]

Any other ideas besides "build"? "packaging"? "infrastructure"? "buildsystem"?

[fbissey]

"selfhosted" but I recon it is not as intuitive as "build". On the other hand

+        sage: (out, err, ret) = test_executable(["sage", "--root"])  # optional - build
+        sage: len(out) >= 2   # at least one character + newline; optional - build

is not exactly related to the build system. It happens to not necessarily be defined on sage-on-distro.

[fbissey]

Speaking of stuff that is not self evident, we may want to add a bit of documentation somewhere to explain what it is.

[fbissey]

Replying to fbissey:

Speaking of stuff that is not self evident, we may want to add a bit of documentation somewhere to explain what it is.

Forget that. I re-read the branch and it is documented well enough.

[embray]

I'm a little lost in the bikeshedding. Does this need work or not? And if so please transition the ticket's state.

[fbissey]

Let's move the stuff that's already done here merged. We can do more bikeshedding some other place.

[embray]

Sounds good to me. If anyone has follow-up concerns please open tickets for them and CC me.

[slabbe]

Few "build" tags were missing. Follow-up at #27781.