OCamlPro / gnucobol

A clone of the sourceforge GnuCOBOL compiler from COBOL to C.
https://get-superbol.com
GNU Lesser General Public License v3.0
16 stars 21 forks source link

[WIP] Provide Autoconf Testsuite syntax for NIST tests #83

Open lefessan opened 1 year ago

lefessan commented 1 year ago

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

lefessan commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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

GitMensch commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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?

lefessan commented 1 year ago

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 commented 1 year ago

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

GitMensch commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

This new version checks the REPORT file after the test.

I still have two weird results:

GitMensch commented 1 year ago

I still have two weird results

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

Additional notes:

lefessan commented 1 year ago
  • 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"...

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 commented 1 year ago

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 commented 1 year ago

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.