[WIP] Provide Autoconf Testsuite syntax for NIST tests

[lefessan]

Using this suite, it is possible to parallelize its evaluation (for example with autofonce run -t tests/cobol85/nist.at)

[GitMensch]

The general idea is good and the approach quite well.

Things to do:

• rename folder "testsuite.src" to "nistrun.src"
• Makefile.am:
  • add the new testsuite for generation and distribution, see how it is done for the testuite one directory above
  • new target check-nist, with dependencies nistrun $(MODULES_ALL) (this will ensure that both the generated testsuite source and the unpacked source files are ready)
• either add run_DBNOIX.at and friend and make the complete include conditional depending on the "with_db" variable or - preferably - check the differences and add the appropriate AT_SKIP_IF for the tests to skip
• testsuite source files:
  • change the complete testsuite to not cp anything, but compile from those directories
  • drop the group from the AT_SETUP as we have that in AT_KEYWORDS
  • drop the -x -debug from AT_CHECK - they are already part of $COMPILE
  • add check of stdout and stderr to the compiles, should be completely empty - for some of the early tests add the appropriate -Wno-goto-section-stuff to the COMPILE
  • actual checking for the expected results (mostly from generated report output, in some places from stdout/stderr) -> see report.pl

This way the general test running should be fine (one test is special as it needs to be killed, just skip it for now), but the important part is the verification (report.pl does some tests, mostly on the generated reports) - I guess that will lead to more AT_CHECK using $GREP/$SED/$AWK, right?

[lefessan]

I don't think I will have time to do all these changes for a full integration in GnuCOBOL in the short term.

My short term plan was more to merge this into the gcos4gnucobol-3.x branch, so that it would be easy to test all the pull-requests.

Anyway, I will try to see what I can do on the short term.

add check of stdout and stderr to the compiles, should be completely empty

Actually, I still get a lot of warnings. I pushed a version where the warnings are present, even with options -Wno-goto-different-section -Wno-goto-section -Wno-obsolete

[lefessan]

Also, I am not sure of what you mean by removing the cp, as the files from NIST have to be downloaded and extracted anyway, I cannot really include them in the testsuite directly.

[GitMensch]

Pass their directories to cobc instead of cp and then compile locally.

[GitMensch]

Sounds good, I can have a look when you consider you are as fast as possible. I now remember that the obsolete warnings are even part of some tests. Note for the pull requests that we already can run `make test --jobs=4` to run the modules in parallel and can do the same for make checkall.

[lefessan]

I have created an issue to keep track of this discussion.

This week-end, I am going to merge this PR in the branch gcos4gnucobol-3.x and release v0.6 of autofonce to work with it. After that, my plan is to move back to the COPY/REPLACING PR to have it merged as soon as possible, now that I can run the testsuite more easily :-)

[GitMensch]

I'd suggest to not merge that yet... Running it that way does miss the actual checking of the test results (you only get "does it compile and run").
Where is the issue with running make checkall --jobs=4 TESTSUITEFLAGS="--jobs=6" to run all in parallel?

[GitMensch]

As this warning is not tested in the NIST testsuite - disable it with -Wno-goto-different-section (for all tests).

[GitMensch]

Better:

-AT_CHECK([cp -f ${abs_builddir}/cobol85/SM/SM208A.CBL .], [0], [], [])
-AT_CHECK([$COMPILE -std=cobol85 -I ${abs_builddir}/cobol85/copy SM208A.CBL], [0], [], [SM208A.CBL: in section 'SECT-SM208A-001':
+AT_CHECK([$COMPILE -std=cobol85 -Wno-goto-different-section -I ${abs_builddir}/cobol85/copy ${abs_builddir}/cobol85/SM/SM208A.CBL], [0], [], [])

But I'd actually suggest to at least define an additional COMPILE85 as $COMPILE -std=cobol85 -I ${abs_builddir}/cobol85/copy - or even a macro so one could use

NIST_COMPILE([SM208A])

[lefessan]

I'd suggest to not merge that yet...

I am not in a hurry, I am happy with the status of autofonce for now, and I can run these tests even if they are not committed in GIT.

Running it that way does miss the actual checking of the test results (you only get "does it compile and run").

I don't understand what you mean: the AT_CHECK actually also check the stdout/stderr. Or do you mean that we should also check generated files ? It's not clear from the report.pl script what they are expected to be. Or is it written somewhere else, like in the tests themselves ?

Where is the issue with running make checkall --jobs=4 TESTSUITEFLAGS="--jobs=6" to run all in parallel?

Of course, I could keep that as an alias, but I don't like having two different testsuites with two completely different ways of running tests. At the end, when a test fails, it is not clear in which environment it was run.

It's a bit of a theoretical problem, as I have not really encountered a case where I would be stuck. Anyway, you know what it is, I wrote a tool, now I want to use it for everything...

[codecov-commenter]

Codecov Report

Merging #83 (cac0406) into gcos4gnucobol-3.x (d4f364e) will not change coverage.
The diff coverage is n/a.

📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more

@@                Coverage Diff                 @@
##           gcos4gnucobol-3.x      #83   +/-   ##
==================================================
  Coverage              65.27%   65.27%           
==================================================
  Files                     31       31           
  Lines                  56542    56542           
  Branches               14767    14767           
==================================================
  Hits                   36908    36908           
  Misses                 13828    13828           
  Partials                5806     5806           

Impacted Files	Coverage Δ
cobc/cobc.c	70.70% <0.00%> (ø)

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

[GitMensch]

Running it that way does miss the actual checking of the test results (you only get "does it compile and run").

I don't understand what you mean: the AT_CHECK actually also check the stdout/stderr. Or do you mean that we should also check generated files ? It's not clear from the report.pl script what they are expected to be. Or is it written somewhere else, like in the tests themselves ?

The tests have the expectations in text form, report.pl has that already in code and checks the resulting report.log (in rare cases something else, like stdout/stderr). Those checks would have to be placed in AT_CHECK, too (they are actually the core of the tests, and this is currently missing).

[lefessan]

If I understand correctly, every test ./<TEST> generates a file REPORT, that should be compared to the file <TEST>.log in a final AT_CHECK() report ? I am going to investigate that...

[GitMensch]

If I understand correctly, every test ./<TEST> generates a file REPORT

yes

that should be compared to the file <TEST>.log in a final AT_CHECK() report ?

no

the .log is not to be compared but to be inspected.
Back in the times when NIST85 was written all those logs were inspected, possibly after printing, to check the results.

report.pl does this from the perl side and writes the results to a summary file (along with a duration.log) per module.
The makefile for each folder then compares the module summary with the expected result file which is in tests/cobol85.

Then finally the summary.pl file reads those module files to write a summary file, which is then compared to the summary/summarynoix in the end.

The summary.pl is mostly useful for a quick validation/telling, it doesn't need to be inspected for this PR. But the report log files are to be inspected after each test run.

Note: dropping the cp `to clean this up is easy, isn't it?

[lefessan]

Note: dropping the cp `to clean this up is easy, isn't it?

I would prefer not because using inplace files may cause the compiler to generate warnings/errors with the absolute position of the file, and it will be different for different users, so the [stdout] and [stderr] arguments of AT_CHECK would become user specific (that's why my user name would appear in the .at files at some point...).

Copying is a bit more expensive, but it makes all the errors local. Also, when debugging a failure, having the COBOL source in the test directory makes it easier to inspect all the files.

[lefessan]

This new version checks the REPORT file after the test.

I still have two weird results:

• AT_CHECK([./IX119A], [1], [], [libcob: IX119A.SUB:408: error: stack overflow, possible PERFORM depth exceeded
• AT_CHECK([./RL207A], [1], [], [libcob: RL207A.SUB:456: error: 'XRECORD-NUMBER' (Type: NUMERIC DISPLAY) not numeric: '\000\000\000\000\000\000'

[GitMensch]

I still have two weird results

I'd suggest to debug those :-) I'm definitely keen to know where those come from.

Additional notes:

• the next big missing thing would be to either also include DBNOIX or to skip some of the DB module and all of IX module if no ISAM is available
• it would likely be reasonable to rename nist.at to nistrun.at, to match the source directory
• "nothing to do, just to recognize: the generated nistrun testscript would need to be executed directly from tests, as that is the place where it is looking for atconfig and atlocal; either "directly" or passing the path to abs_builddir/test via its -C option
• one thing I've wondered: is there a reason to not include the following directly in nist[run].at?

  export COB_DISABLE_WARNINGS=Y
  export COB_SWITCH_1=ON
  export COB_SWITCH_2=OFF
  export COMPILE85="$COMPILE -std=cobol85 -Wno-goto-different-section -Wno-goto-section -I copy"
  export COMPILE_MODULE85="$COMPILE_MODULE -std=cobol85 -Wno-goto-different-section -Wno-goto-section -I copy"
  export GREP="$GREP --text"

  Note: AC_CHECK likely creates a sub-shell so this must be "outside"...
• somehow ar-lib got into this PR again (was the case with previous ones, too)

[lefessan]

• one thing I've wondered: is there a reason to not include the following directly in nist[run].at?

  ```shell
  export COB_DISABLE_WARNINGS=Y
  export COB_SWITCH_1=ON
  export COB_SWITCH_2=OFF
  export COMPILE85="$COMPILE -std=cobol85 -Wno-goto-different-section -Wno-goto-section -I copy"
  export COMPILE_MODULE85="$COMPILE_MODULE -std=cobol85 -Wno-goto-different-section -Wno-goto-section -I copy"
  export GREP="$GREP --text"
  ```
  
  Note: `AC_CHECK` likely creates a sub-shell so this must be "outside"...
  

I couldn't find a way to do that. It looks like the only way to configure globally the testsuite is to add this in atlocal, except that it would make the standard testsuite fail, since there is no way to test from atlocal which testsuite is being executed.

[lefessan]

My guess is that the solution would be to keep nistrun.at in the cobol85 sub-directory, so that it will have its own atconfig/atlocal files different from the ones in tests.

[GitMensch]

I couldn't find a way to do that. It looks like the only way to configure globally the testsuite is to add this in atlocal, except that it would make the standard testsuite fail, since there is no way to test from atlocal which testsuite is being executed.

Oh, it is. atlocal is sourced, so we have the full running environment in there

if test "$(basename "$0")" = "nistrun"; then
   export COB_DISABLE_WARNINGS=Y
   export COB_SWITCH_1=ON
   export COB_SWITCH_2=OFF
   export COMPILE85="$COMPILE -std=cobol85 -Wno-goto-different-section -Wno-goto-section -I copy"
   export COMPILE_MODULE85="$COMPILE_MODULE -std=cobol85 -Wno-goto-different-section -Wno-goto-section -I copy"
   export GREP="$GREP --text"
fi

I think that the first three likely would be better set directly in the specific tests that need those:

AT_CHECK([COB_SWITCH_1=ON COB_SWITCH_2=OFF \
$COBCRUN_DIRECT ./RW101A], [0], [], [])

note: please add the $COBCRUN_DIRECT as this is an optional test runner - one can use that to do calls to valgrind, perf and other tools.