gnucobol icon indicating copy to clipboard operation
gnucobol copied to clipboard

[WIP] Provide Autoconf Testsuite syntax for NIST tests

Open lefessan opened this issue 2 years ago • 17 comments

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

lefessan avatar Feb 03 '23 20:02 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 avatar Feb 04 '23 00:02 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.

lefessan avatar Feb 04 '23 00:02 lefessan

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

GitMensch avatar Feb 04 '23 08:02 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.

GitMensch avatar Feb 04 '23 08:02 GitMensch

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 :-)

lefessan avatar Feb 04 '23 16:02 lefessan

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 avatar Feb 04 '23 18:02 GitMensch

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...

lefessan avatar Feb 05 '23 22:02 lefessan

Codecov Report

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

:mega: 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%> (ø)

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

codecov-commenter avatar Feb 05 '23 22:02 codecov-commenter

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).

GitMensch avatar Feb 05 '23 22:02 GitMensch

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...

lefessan avatar Feb 06 '23 09:02 lefessan

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 <TEST>.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?

GitMensch avatar Feb 06 '23 11:02 GitMensch

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 avatar Feb 06 '23 12:02 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'

lefessan avatar Feb 06 '23 21:02 lefessan

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)

GitMensch avatar Feb 06 '23 22:02 GitMensch

  • 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 avatar Feb 09 '23 21:02 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.

lefessan avatar Feb 10 '23 16:02 lefessan

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.

GitMensch avatar Feb 10 '23 19:02 GitMensch