Built-in tests should not run pre-test and post-test blocks

[ryandesign (Ryan Carsten Schmidt)]

The test phase now runs built-in tests for all ports, and CI has been modified to run only the built-in tests. This fails on ports that have pre-test or post-test blocks because those blocks were all written at a time when built-in tests did not exist, so of course they assume that the test dependencies have been installed and (in the case of the post-test block) that the test command has been run.

Some failures caused by this:

• ​https://github.com/macports/macports-ports/pull/23485
• ​https://github.com/macports/macports-ports/pull/23764

Josh suggested that each port could check, in its pre- and post-test blocks, whether test.run is yes, but I don't think all ports with such blocks should have to be modified for this base change. Instead, base should not run pre- and post-test blocks for the built-in tests.

Perhaps the build-in tests should not be part of the test phase at all. Perhaps they should be a completely separate phase that can be requested separately by CI. And the test phase could have a dependency on this new separate phase to keep the existing behavior.

[jmroot (Joshua Root)]

This isn't really something that happens outside of CI because there's no official interface to allow only running the built-in tests. Setting test.run=no on the command line is a hack.

[ryandesign (Ryan Carsten Schmidt)]

The fact that it happens in CI is sufficient justification to fix it.

And, great: moving the built-in tests to a separate phase will not only fix the CI test failures but will also provide the official interface to run the built-in tests and eliminate the need for a hack.

[jmroot (Joshua Root)]

Alternatively, we could run the test phase normally on CI and deal with ports that take an excessively long time to test as they come up.

[ryandesign (Ryan Carsten Schmidt)]

One port that took a long time to test was curl which was recently fixed by upstream improvements to allow testing in parallel.

It seems very likely to me though that a lot of ports have test suites that don't pass. Are we going to fail the CI for that reason? And not even try the destroot phase to see if it would succeed? It's not our responsibility to fix upstream test suites. I think it will make it more likely that people submit fixes that have nothing to do with the test suite, the test suite fails, we then don't know if destroot will succeed, and we either leave the PR open forever because of the uncertainty or we merge it without verifying whether it actually installs.

[jmroot (Joshua Root)]

CI installs the port first (with -k) and then runs port test as a separate step. We don't have to make the indicator go red if the test phase fails. It should certainly be a warning annotation at the very least though.

[ryandesign (Ryan Carsten Schmidt)]

Replying to jmroot:

We don't have to make the indicator go red if the test phase fails.

We currently do, though.