https://bz.apache.org/SpamAssassin/show_bug.cgi?id=7988

            Bug ID: 7988
           Summary: Fix and streamline tests for release process
           Product: Spamassassin
           Version: 4.0.0
          Hardware: All
                OS: All
            Status: NEW
          Severity: normal
          Priority: P2
         Component: Regression Tests
          Assignee: dev@spamassassin.apache.org
          Reporter: sid...@sidney.com
  Target Milestone: Undefined

In preparing the 4.0.0 release I have come across some bugs and some
difficulties in running the tests in t and xt directories. I'm opening this
issue to fix all of them at once, but here is a list that can be discussed as
separate items:

1. The xt directory does not fulfill its purpose and is hard to maintain.

The commit message for its creation says that it is supposed to ensure that the
release manager runs all the tests. This is because the default settings in
t/config.dist skip certain tests if you just run make test. The intention is
that the release manager can run make test TEST_FILES="xt/*.t" and cause all
tests in t/*.t to be run without skipping the tests the are disabled by
default.

To implement this, each test in xt/*.t calls its corresponding test in t/ with
a flag override if the test is one that depends on a flag in t/config.dist

One problem with this is that it is hard to maintain. Every time a new test is
added in t/ the corresponding test has to be added to xt/. Also, some tests,
such as dcc.t depend on two flags, and it is easy to introduce a bug by not
overriding all the necessary flags when writing the test in xt/.

I propose to write a shell script named something like xt/release_test_suite.sh
that runs all files it finds in t/*.t with all the suitable option flags
enabled. This will serve the purpose of the xt directory without the
maintainability problems. All the small xt/*.t files that just call a file in
t/*.t can be deleted,

This isn't portable to Windows, but the target user of the script means it
doesn't have to be for now. It doesn't remove the ability of anyone running
Windows to run a test that they can now.

2. There are three test files in xt/ that are not mirrors of tests in t/*.t
These three are meant to be of interest only to release managers and packagers.
They would also be run by the new script.

3. There is some bit rot in the three xt-specific test scripts, both from
changes that have been made to t/SATest.pm and other code changes since 3.4
that were done with nobody running the xt tests. xt/20_saw_ampersand.t is
particularly bad because it can only be run in an ancient Perl version less
than 5.17, so it doesn't see much testing.

4. In testing the tests, I found that they don't work if run outside of using
make test, for example by running with the prove command. The problem results
from SATest.pm putting relative paths on @INC which then breaks if a test
changes directories. The setup of @INC in the Makefile for a test happens to
use the correct absolute pathname so running make test hides the bug in SATest.
Also, some tests are not calling SATest to initialize their environment, and
they only happen to work because Makefile sets up enough to work, which doesn't
happen if the test is run using prove.

5. There are 11 test files t/root*.t that are intended to be run using
something like sudo make test TEST_FILES="t/root*.t"
Currently they will only run if the uid is 0 (being run as root), and if a flag
in t/config.dist is set to enable the root tests.

I don't think the flag in config.dist is necessary. Nobody is likely to run the
root tests as root without intending to. I'm more concerned about someone who
doesn't realize they are in a root shell running all the non-root tests. I
would remove the run_root_tests flag from config.dist and add an optional
argument to sa_t_init() in SATest.pm that the t/root*.t tests can use to tell
sa_t_init that it is ok to run as root, having sa_t_init() otherwise fail if
running as root.

I already have fixes for 1-4 ready to commit. I can add 5 as a separate commit,
since that it pretty independent.

-- 
You are receiving this mail because:
You are the assignee for the bug.

Reply via email to