The action should fail if the test file doesn't exist

[wilfwilson]

Currently that's not happening, see e.g. https://github.com/gap-packages/anupq/runs/2315066518?check_suite_focus=true where the tests are 'passing' even though the test file can't be found.

[wilfwilson]

I completely forgot that I posted this issue (only 8 days ago!), such that I was about to create a duplicate of this issue after encountering it again in a different context!

I'm not sure whether this is rightly a problem of https://github.com/gap-system/gap or https://github.com/gap-system/pkg-ci-scripts or here, but I'm putting it here because this is the actual place where I'm getting the problem.

This run-pkg-tests action runs pkg-ci-scripts/run_tests.sh https://github.com/gap-system/pkg-ci-scripts/blob/448511592a4e32b36571e6f8e4cbf0e8d440c34b/run_tests.sh#L38 which attempts to run a test file by including it as an argument to gap.sh.

The problem is that bin/gap.sh --quitonbreak tst/testfail.g, where tst/testfail.g does not exist, does not return 0; indeed no break loop occurs (I'm not even sure how GAP quits in this case?). Therefore GitHub Actions workflow assumes that everything worked fine.

See:

$ bin/gap.sh -A --quitonbreak tst/testfail.g
 ┌───────┐   GAP 4.12dev-855-g84fd537 built on 2021-05-06 19:16:27+0100
 │  GAP  │   https://www.gap-system.org
 └───────┘   Architecture: x86_64-apple-darwin20.4.0-default64-kv8
 Configuration:  gmp 6.2.1, GASMAN, readline
 Loading the library and packages ...
 Packages:   GAPDoc 1.dev, PrimGrp 3.4.1, SmallGrp 1.4.2, TransGrp 3.1
 Try '??help' for help. See also '?copyright', '?cite' and '?authors'
Could not read file "tst/testfail.g".
gap>

Note that using the -c option gives the desired behaviour:

$ bin/gap.sh -A --quitonbreak -c 'Read("tst/testfail.g");'
 ┌───────┐   GAP 4.12dev-855-g84fd537 built on 2021-05-06 19:16:27+0100
 │  GAP  │   https://www.gap-system.org
 └───────┘   Architecture: x86_64-apple-darwin20.4.0-default64-kv8
 Configuration:  gmp 6.2.1, GASMAN, readline
 Loading the library and packages ...
ech Packages:   GAPDoc 1.dev, PrimGrp 3.4.1, SmallGrp 1.4.2, TransGrp 3.1
 Try '??help' for help. See also '?copyright', '?cite' and '?authors'
oError, file "tst/testfail.g" must exist and be readable at /Users/Wilf/gap/lib/files.gi:210 called from

$ echo $?
1

(Why does it say oError?!)

But -c isn't supported in older versions of GAP so this is probably not an acceptable solution.

Perhaps run_tests.sh should check for the existence of the $GAP_TESTFILE before calling GAP?

[fingolfin]

@Joseph-Edwards perhaps you'd be interested in taking a stab at resolving this issue, too? It shouldn't be too hard (just adding a IsExistingFile(GAP_TESTFILE) test in the right place perhaps?)

[Joseph-Edwards]

@Joseph-Edwards perhaps you'd be interested in taking a stab at resolving this issue, too? It shouldn't be too hard (just adding a IsExistingFile(GAP_TESTFILE) test in the right place perhaps?)

Sounds good; I'll give it a look when I have a moment.

[Joseph-Edwards]

I think that this issue doesn't exist any more, as we non longer depend on pkg-ci-scripts. If the file specified by GAP_TESTFILE is empty or non-existent, then the following lines set GAP_TESTFILE to info.TestFile:

run-pkg-tests/action.yml

Lines 76 to 81 in e1d40f3

	GAP_TESTFILE:="${{ inputs.GAP_TESTFILE }}";
	Read("PackageInfo.g");
	info := GAPInfo.PackageInfoCurrent;
	if IsEmpty(GAP_TESTFILE) or not IsExistingFile(GAP_TESTFILE) then
	GAP_TESTFILE := info.TestFile;
	fi;

If info.TestFile is either unspecified, or points to a non-existent file, the CI fails for other reasons. See this sample PR for the example package.

@fingolfin, I think this issue can be closed, unless you think any further action is necessary.

[fingolfin]

Closing this now (CC @wilfwilson -- please reopen / complain if you think this is not right)

[wilfwilson]

Very nice.

[wilfwilson]

Actually, to be honest I would not quite consider the current situation to be good enough. Silently doing something other than what the user requested (if they actually went to the effort of specifying a particular test file!) seems non-ideal. I'll re-open and sort it out myself.