Network access during testing vs autobuilders

[barak]

This is an issue on version 1.5.1.0, which I package for Debian. When it builds on the Debian autobuilders, there is a failure in test cases that try to access the network by binding a port. See transcript below, or here for complete build transcripts across various architectures.

The obvious fix would be to have a NO_NETWORK option that disables anything in the build process, including during testing, that requires network access or binds ports.

> ../chuck 01-Basic/113.ck
Retrying test: ../chuck 01-Basic/113.ck
> ../chuck 01-Basic/113.ck
Retrying test: ../chuck 01-Basic/113.ck
> ../chuck 01-Basic/113.ck
*** Test '113.ck' failed: ***
[chuck]: cannot bind to tcp port 8888...
null null 

> ../chuck 01-Basic/114.ck
Retrying test: ../chuck 01-Basic/114.ck
> ../chuck 01-Basic/114.ck
Retrying test: ../chuck 01-Basic/114.ck
> ../chuck 01-Basic/114.ck
*** Test '114.ck' failed: ***
[chuck]: cannot bind to tcp port 8888...
114.ck:2:6: error: cannot '=>' from/to a multi-variable declaration
[2] 5 => int x, y;
         ^

[gewang]

Hi @barak thanks for letting us know (and thank you so much for the work in packaging chuck for Debian). The cannot bind to tcp port 8888... message happens when there is another chuck process (CLI chuck or miniAudicle) currently running. I wonder if this is somehow a parallelization issue? If you run the tests again, do you get the same errors?

(also cc @nshaheed could this be related to the -j flag we added to make a while back?)

[barak]

Oh, that's a possibility I had not considered, and is consistent with the build failing on all "official" architectures which have big-beast-boxes and highly parallel builds, but succeeding on all the others, which have shoestring budgets and probably build single threaded. Is the testing parallel?

In any case, the test failing if something—another chuck process, or even something else—having port 8888 bound does seem like a bug. Like, people often put http servers on port 8888.

For now, as a possible workaround, I will disable parallelism in the testing process. If that fails to solve the problem I'll get back to you. But I'd suggest keeping this issue open pending a more principled solution.

[barak]

Okay, I think I found the issue. There's a bug in the test target of src/makefile which ignores test errors.

But I am applying a patch to src/makefile to fix a bash-ism which also fixed the bug.

$ git show patch-queue/debian 
commit 274ed97f291c12ec9cdcd173b6ac0522455a9a16 (HEAD -> patch-queue/debian)
Author: Barak A. Pearlmutter <barak+git@pearlmutter.net>
Date:   Tue Jul 4 09:59:04 2023 +0100

    test fix

    Address two issues in the test target in src/makefile:
    - POSIX shell does not mandate pushd.
    - Exit gracefully if directory does not exist or python script fails.

    Gbp-Pq: Name 0003-test-fix.patch

diff --git a/src/makefile b/src/makefile
index a72edacb..27375ba5 100644
--- a/src/makefile
+++ b/src/makefile
@@ -261,7 +261,7 @@ $(CXXOBJS_HOST): %.o: %.cpp

 ############################## RUN UNIT TEST ##################################
 test:
-       pushd test; ./test.py ../chuck .; popd
+       (cd test && ./test.py ../chuck .)

 ############################### DISTRIBUTION ##################################

Notes:

• The command sequence foo ; bar runs bar and fails iff bar fails, regardless of whether foo gets an error or not. But foo && bar fails without running bar if foo fails.
• pushd and popd are bash-isms, not supported by the Debian-default Bourne shell /bin/sh which is what make invokes by default, so make test was getting an error ... from the popd. Which should be a no-op anyway since the subshell is being terminated at the end of the line, but whatever.

Example:

$ (exit 1)
$ echo $?
1
$ (exit 1) ; echo foo
foo
$ echo $?
0
$ (exit 1)  && echo foo
$ echo $?
1

tldr: Failure of test.py was being ignored by the test target in src/makefile and I fixed that bug, thus exposing these test errors.

[gewang]

thank you for getting to the bottom of this! would you recommend adopting this more shell-agnostic approach in general (I only see benefits)? would changing the test target's action to (cd test && ./test.py ../chuck .) instead of using pushd/popd be all that is needed, or would additional modifications be needed?

[barak]

Yes, I think this change is all that is needed, and has no cons.

The reason for enclosing it in parens is just for clarity, to make it extra clear that the directory change has limited scope. The subshell should disappear after the end of the line anyway, so they aren't really necessary. Well, unless something obscure like .ONESHELL: is in play.

[gewang]

pushed in 2295db4; will be part of chuck-1.5.1.1. Thank you @barak, as always!