Integrate tests into automake test suite

The ntor, backtrace and zero lengths keys tests are not integrated into the automake test suite. This means these tests not being include in the test results (with PASS/FAIL/etc), clutter up the terminal with their own output, and are potentially not being run parallel.

changed milestone to %Tor: 0.2.7.x-final in legacy/trac

added 027-triaged-1-in in Legacy / Trac SponsorS in Legacy / Trac component::core tor/tor in Legacy / Trac milestone::Tor: 0.2.7.x-final in Legacy / Trac owner::cypherpunks in Legacy / Trac points::small in Legacy / Trac priority::very low in Legacy / Trac resolution::fixed in Legacy / Trac status::closed in Legacy / Trac testing in Legacy / Trac type::enhancement in Legacy / Trac version::tor 0.2.7 in Legacy / Trac labels

Is it expected behavior that the backtrace test does not communicate its failure?

for I in range(len(LINES)):
    if matches(LINES[I:], FUNCNAMES):
        print("OK")
        break
else:
    print("BAD")

The snippet above from bt_test.py only prints OK or BAD but does not change its exit code so the make rule where it is called never fails.

Trac:
Owner: N/A to cypherpunks
Status: new to assigned

Trac:
Milestone: Tor: unspecified to Tor: 0.2.7.x-final

No, it would be good if it had a nonzero exit code in that case.

See legacy/trac#11659 (moved) for a related ticket.

Trac:
0001-Integrate-zero_length_keys-test-into-the-automake-te.patch

Trac:
0002-Integrate-ntor-test-into-the-automake-test-suite.patch

Trac:
0003-Integrate-backtrace-test-into-the-automake-test-suit.patch

Trac:
0004-Add-changes-file-for-15344.patch

For documentation purposes, @abs_top_srcdir@ is used deliberately in test_ntor.sh.in and test_bt.sh.in because autoconf ignores the non-recursive nature of Tor. Variable substitutions of relative paths are computed from the directory the substituted files are located in, but tests are executed from the build directory root. For example, with test_ntor.sh the top_srcdir variable would result in ../../src/test. When executed from the build directory root it is obvious that this relative path won't find the files it is looking for. Therefore we use absolute paths.

Trac:
Status: assigned to needs_review

Marking more tickets as triaged-in for 0.2.7

Trac:
Keywords: N/A deleted, 027-triaged-1-in added

I wonder if there's some way to do this without moving all our scripts into getting pre-processed by autoconf.

Trac:
Version: Tor: unspecified to Tor: 0.2.7
Keywords: N/A deleted, SponsorS added
Priority: minor to trivial
Points: N/A to small

Replying to nickm:

I wonder if there's some way to do this without moving all our scripts into getting pre-processed by autoconf. I have not found a way to pipe output from one command to another without using wrapper scripts as in the ntor and backtrace tests. With regards to the zero length test, i agree it is ugly to have the entire script be processed by autoconf.

A solution would be to give the path to the Tor binary as a parameter so it does not need to be hard coded into the script. As for the shell substitution (@SHELL@), it can be removed if the script does not need to call itself anymore. This be achieved by changing some logic within the script. These solutions would still require a wrapper script (similar to the ntor and backtrace tests) that calls the zero length script with the appropriate Tor binary but at least the zero length script itself would not require any pre-processing.

As final remarks, i think we will always need some pre-processing in the tests if we want to support out-of-tree builds properly. The best solution i can currently come up with is to concentrate the pre-processing to the wrapper scripts.

Expect a fixup patch in a couple of days.

Trac:
0001-Integrate-zero_length_keys-test-into-the-automake-te.2.patch

Trac:
0002-Integrate-ntor-test-into-the-automake-test-suite.2.patch

Trac:
0003-Integrate-backtrace-test-into-the-automake-test-suit.2.patch

A fixup patch did not apply cleanly so i created a separate branch (hence the new patches). I took this opportunity to rebase the patches on the current master (0693955) and clean them up (there were some unintentional file mode changes). The old patches 0001, 0002 and 0003 are now deprecated, but patch 0004 can be reused as it only adds the changes file.

The new approach for the zero lengths keys test is described in its commit message.

Merged to master; thanks!

Trac:
Resolution: N/A to implemented
Status: needs_review to closed

Hm. Apparently this breaks with older automakes and makes our tests not pass on Debian stable. I should fix that...

Trac:
Resolution: implemented to N/A
Status: closed to reopened

d579d751 may fix this...

Indeed, it seems to do so.

Trac:
Status: reopened to closed
Resolution: N/A to fixed

Sorry for the incomplete patch. I tend to focus on portability but not so much on backwards compatibility. Great fix, i couldn't find the problem myself. However, i have two remarks on your fixes. Do with it what you want, it's nothing major.

1. commit ecf98313 doesn't do anything but make unprocessed files executable. AFAIK it didn't contribute in fixing the problem and smells like bad practice. Therefore, IMO the commit should be reverted.
2. all of the files generated in the build directory have permissions 600 (or 700 for executable files). For consistency the wrapper scripts should also be 700 (or maybe use chmod u+x).

Just my two (remark) cents.

Reverted that commit.

Trying it with u+x.

closed

mentioned in issue legacy/trac#15818 (moved)

mentioned in issue legacy/trac#17859 (moved)

moved from legacy/trac#15344 (moved)

added Feature label and removed 1 deleted label

added Tests label and removed 1 deleted label