The Cobol test runner needs a little more check

[Borderite]

I have been solving exercises in the Cobol track for the past few weeks. While looking at community solutions, I noticed that some solutions passed all tests despite that they do not yield rignt answers. So, I have investigated what was going on.

It seems to me that the cause of the problem is very simlple. The tests used in each exercise is a cobol program generated by Cobol Check, which combines the information from the “tut” file and the student’s solution code. But a cobol program can be NORMALLY terminated by the “stop run” or “goback” statement anywhere in the procedure division. If this happens, the test executable terminates regardless of whether or not any test has been performed, and the return code is zero. The test-runner seems to take the zero return code as the sign of passing all tests.

Unfortunately, a typical introductry Cobol course teaches students that “stop run” should be put at the end of the main paragraph. Some students at Exercism naturally practice this teaching. This means that the test runner needs to inspect the output from the test and verifies that the solution passes ALL tests.

I am wondering if I am missing something.

[mk-mxp]

You are right, and the problem was discussed multiple times in the forum. kotp provided a possible solution, that has not yet found its way into the test runner.

That said, maybe you could add such a check to the test runner? @axtens would surely love to see such a fix.

[axtens]

You’re not missing anything. This is a known issue and I just haven’t had the time. All it probably needs is a extra check at the very end of the test-runner to check for the presence of these keywords.

The other thing that might be done is to tweak cobol-check itself, but I’m too faint-hearted for that (and I don’t know Java well).

@mk-mxp is correct, I would love to see this fixed. It grieves me that students are getting the wrong idea about their ability to code cobol.

Folk whom I’ve mentored I instructed to leave out the offending commands.

[Borderite]

@mk-mxp, @axtens,

Thanks for your responses. I have gone over a test code generated by Cobol Check. It seems that the code reaches the final part in which the summary numbers are displayed, as long as the solution code does not terminate the tests by goback or stop run. Isn’t it enough to verify the last four lines of the output are like this:

 24 TEST CASES WERE EXECUTED
 24 PASSED
  0 FAILED
=================================================

If you agree with my assessment, I can modify run.sh of the cobol test runner along this line.

[axtens]

At one level, yes. But the student should be able to know immediately why. So at that level, no. I think it would be better to actually sniff the source for stop run and goback and tell the student politely that we don’t do that kind of thing around here.

[Borderite]

@axtens I don’t think that it is satisfactory to tell students not to use “stop run” and “goback” politely. They need to know that the cobol track’s tests use Cobol Check and what Cobol Check does. Without such knowledge, for example, a student may mistakenly expect variables to be initialized in data division in every test.

I think that you might want to add the information to the " Testing on the COBOL Track" page. Then the error message could provides a link to the page.

[Borderite]

@mk-mxp, @axtens

I wrote up a brief description on COBOL CHECK. Should we put it in the Tests page of the COBOL track, it would look like what you see below. Please let me know how you feel about this revision.

Once the revision is done, I can revise the test runner, so that it rejects incomplete tests with an error message pointing to the Tests page.

=================================================

Tests

The tests in the COBOL track use a software called COBOL CHECK created by Dave Nicolette and later improved in collaboration with the Open Mainframe Project. While you can find much detailed information on COBOL CHECK by visiting its GitHub page and following the links provided there, we here only briefly describe how it works to facilitate your maneuver through the exercises. If you are already familier with COBOL CHECK, you can skip to Running the Tests

COBOL CHECK

COBOL CHECK was designed for conveniently testing individual paragraphs in the procedure division. To attain this goal, COBOL CHECK reads a target COBOL program and add several lines of code to it for providing test data, executing paragraphs and checking the results the paragraphs generate.

For example, suppose that you solve an exercise in which you write a paragraph ADD-TWO-NUMBERS that calculates the sum of two numbers. Besides the COBOL program containing the ADD-TWO-NUMBERS paragraph, COBOL CHECK requires a file of test descriptions written in a COBOL-like domain specific language. For example:

TESTSUITE "Add Two Numbers"

TestCase "adding zeros gives zero."
  MOVE 0 TO WS-NUMBER-1 
  MOVE 0 TO WS-NUMBER-2
  PERFORM ADD-TWO-NUMBERS
  EXPECT WS-SUM = 0
  
TestCase "adding 3 to 5 yields 8."
  MOVE 3 TO WS-NUMBER-1 
  MOVE 5 TO WS-NUMBER-2
  PERFORM ADD-TWO-NUMBERS
  EXPECT WS-SUM = 8

TestCase "adding -8 to 5 yields -3."
  MOVE -8 TO WS-NUMBER-1 
  MOVE 5 TO WS-NUMBER-2
  PERFORM ADD-TWO-NUMBERS
  EXPECT WS-SUM = -3

This file shows:

1. Each test passes numbers to data items XE-NUMBER-1 and XE-NUMBER-2 and expects the result in another data item XE-SUM. So, the three data items must be present in the Data Division of the program to be tested.
2. The ADD-TWO-NUMBERS paragraph has to save the sum it calculates in WS-SUM.

Let’s prepare a solution

       IDENTIFICATION DIVISION. 
       PROGRAM-ID. ADD-TWO-NUMBERS.
       DATA DIVISION.
       WORKING-STORAGE SECTION.
       01 WS-NUMBER-1 PIC S9(4).
       01 WS-NUMBER-2 PIC S9(4).
       01 WS-SUM PIC S9(4) VALUE 0.
       PROCEDURE DIVISION.
       ADD-TWO-NUMBERS.
           ADD WS-NUMBER-1 TO WS-SUM.
           ADD WS-NUMBER-2 TO WS-SUM.

From the test description file and this COBOL program file, COBOL CHECK generates a new COBOL program. Because the actual generated program is pretty complicated and lengthy, a much simplified version is presented here.

       IDENTIFICATION DIVISION. 
       PROGRAM-ID. ADD-TWO-NUMBERS.
       DATA DIVISION.
       WORKING-STORAGE SECTION.
       01 WS-NUMBER-1 PIC S9(4).
       01 WS-NUMBER-2 PIC S9(4).
       01 WS-SUM PIC S9(4) VALUE 0.
       01 CUT-TEST-COUNT PIC 9(3) VALUE 0.
       01 CUT-PASS-COUNT PIC 9(3) VALUE 0.
       01 CUT-FAIL-COUNT PIC 9(3) VALUE 0.
       PROCEDURE DIVISION.
       TEST-1.
           MOVE 0 TO WS-NUMBER-1
           MOVE 0 TO WS-NUMBER-2
           PERFORM ADD-TWO-NUMBERS
           ADD 1 TO CUT-TEST-COUNT
           IF WS-SUM = 0
               ADD 1 TO CUT-PASS-COUNT
               DISPLAY 'TEST ' CUT-TEST-COUNT ' PASSED'
           ELSE
               ADD 1 TO CUT-FAIL-COUNT
               DISPLAY 'TEST ' CUT-TEST-COUNT ' FAILED'
           END-IF.
       TEST-2.
           MOVE 3 TO WS-NUMBER-1
           MOVE 5 TO WS-NUMBER-2
           PERFORM ADD-TWO-NUMBERS
           ADD 1 TO CUT-TEST-COUNT
           IF WS-SUM = 8
               ADD 1 TO CUT-PASS-COUNT
               DISPLAY 'TEST ' CUT-TEST-COUNT ' PASSED'
           ELSE
               ADD 1 TO CUT-FAIL-COUNT
               DISPLAY 'TEST ' CUT-TEST-COUNT ' FAILED'
           END-IF.
       TEST-3.
           MOVE -8 TO WS-NUMBER-1
           MOVE 5 TO WS-NUMBER-2
           PERFORM ADD-TWO-NUMBERS
           ADD 1 TO CUT-TEST-COUNT
           IF WS-SUM = -3
               ADD 1 TO CUT-PASS-COUNT
               DISPLAY 'TEST ' CUT-TEST-COUNT ' PASSED'
           ELSE
               ADD 1 TO CUT-FAIL-COUNT
               DISPLAY 'TEST ' CUT-TEST-COUNT ' FAILED'
           END-IF.
       TEST-SUMMARY.
           DISPLAY CUT-TEST-COUNT ' TEST CASES WERE EXECUTED.'
           DISPLAY CUT-PASS-COUNT ' PASSED'
           DISPLAY CUT-FAIL-COUNT ' FAILED'.
      * Your code follows. 
       ADD-TWO-NUMBERS.
           ADD WS-NUMBER-1 TO WS-SUM
           ADD WS-NUMBER-2 TO WS-SUM.

This code demonstrates:

3. Three tests are performed in execution of a single program. If a data item is modified in the first test, for example, the second test inherits the modified value. You need to be careful about initialization through VALUE clause in the data division. (In the current example, the ADD-TWO-NUMBERS paragraph fails in the third test, WS-SUM being equal to 8 when it is executed after the second test.)
4. MOST IMPORTANTLY, THESTOP RUN AND GOBACK STATEMENTS CANNOT BE USED IN THE CODE TO BE TESTED, because they prematually terminates the entire testing program.

Running the Tests

Unlike the underlying mechanism discussed above, the actual process of running tests is simple.

Choose your current OS:

• Windows
• macOS
• Linux

Windows

1. Open a Command Prompt.

2. Get the first exercise:

    C:\Users\USERNAME> exercism download --exercise hello-world --track cobol
   

3. Change directory into the exercise folder:

    C:\Users\USERNAME> cd C:\Users\USERNAME\exercism\cobol\hello-world
   

4. Run the tests:

    C:\Users\USERNAME\exercism\cobol\hello-world> test.ps1
   

   (Don’t worry if the tests fail when you first run them, this is how you begin each exercise.)

5. Solve the exercise.
   Read through the instructions.md and see how to solve the exercise (view on GitHub).

Good luck, have fun and welcome to the COBOL track!

---

macOS

1. Open a Terminal window.

2. Get the first exercise:

    $ exercism download --exercise hello-world --track cobol
   

3. Change directory into the exercise folder:

    $ cd /Users/USERNAME/exercism/cobol/hello-world
   

4. Run the tests:

    $ bash test.sh
   

   (Don’t worry if the tests fail when you first run them, this is how you begin each exercise.)

   Hint: If you get a message that basename is missing, you can use brew to install coreutils. (brew install coreutils)

5. Solve the exercise.

   Read through the instructions.md and see how to solve the exercise (view on GitHub).

Good luck, have fun and welcome to the COBOL track!

---

Linux

1. Open a Terminal window.

2. Get the first exercise:

    $ exercism download --exercise hello-world --track cobol
   

3. Change directory into the exercise folder:

   $ cd /home/USERNAME/exercism/cobol/hello-world
   

4. Run the tests:

    $ bash test.sh
   

   (Don’t worry if the tests fail when you first run them, this is how you begin each exercise.)

5. Solve the exercise.

   Read through the instructions.md and see how to solve the exercise (view on GitHub).

Good luck, have fun and welcome to the COBOL track!

[axtens]

@Borderite

facilitate your maneuver through the exercises

I would have you submit another PR for these changes. The above is highlighted because it requires a level of English that not all our students attain to. Prefer words from the Anglo-Saxon roots of English over those from Greek and Latin. Phrases like “ease your way through the tasks” or “help your movement through the tasks” would be good.

And, in case I forget later, thank you for taking the time and for including

MOST IMPORTANTLY, THE STOP RUN AND GOBACK STATEMENTS CANNOT BE USED IN THE CODE TO BE TESTED, because they prematurely terminate the entire testing program.

I’m still not sure if we should say why stopping too early is a problem. Do I want to let the students know that that is a good way to have the tests pass?

[Borderite]

@axtens

You must be kidding me. I have been spending most of my life in the United States and have never seen someone who suspects that “facilitate” is a Greek or Latin.

For the last sentence of your response, I would like to remind you that my latest mesage clearly said that I would revise the test runner to reject solutions that do not complete tests.

That said, you can use my writing up in the way you like, if you like. I am just out, because I don’t waste my time.

[BNAndras]

@axtens

I’m still not sure if we should say why stopping too early is a problem. Do I want to let the students know that that is a good way to have the tests pass?

They’re only cheating themselves in the end if they try to circumvent the test runner on purpose.

[BethanyG]

@Borderite

You must be kidding me. I have been spending most of my life in the United States and have never seen someone who suspects that “facilitate” is a Greek or Latin.

I’m sorry @Borderite - I wasn’t going to chime in, but your reply sounded so … aggressive I felt like I had to. I’ve lived in the US my entire life, and am a bit of a word geek.

facilitate isn’t suspected of being Latin - it is Latin in origin. It came to English through Italian and French, but its stem is from the Latin facilis (easy to do).

But I also think that the maintainer of a track gets to set the tone of the writing without others questioning them or shaming them, Please try to be kinder in the future.

[BNAndras]

As Bruce suggested, testing directions should be to the point and easily digestible. You have a global audience with differing levels of English proficiency so that’s something I actively have to watch for in my own tracks’ documentation.

Most of the track-specific testing pages I’ve seen don’t go that much in detail. I don’t think there’s an expectation that the COBOL docs should go in depth so I don’t think you need to explicitly state that you’re not doing that.

[axtens]

@BNAndras

Most of the track-specific testing pages I’ve seen don’t go that much in detail.

And so I begin again on my circumambulation (now that is definitely Latinate) around the problem, which is:

The 3rd-party testing tool gives false positives when the student’s solution contains STOP RUN or GOBACK. Students are currently getting the idea that their code is brilliant when it is usually anything but.

Some people reckon that the behaviour should be dealt with in the documentation. However, not everyone RTFMs (reads the friendly manual). Rather, they’ve read a COBOL manual from 40 years ago and think that’ll be enough.

Other folk think it should be handled in the test-runner, but If it’s to be handled there that’s tricky because it’s not a syntax error: it’s a perfectly legal COBOL statement. Thus I think it could be handled in a separate linting step, probably at the very end. However, this means telling the student right there and then not to use it and I’m currently wondering how to phrase that in a short, succinct manner that doesn’t come over like electroshock therapy.

Which brings me all the round to asking myself (and anyone who’s listening) yet again, how much hand-holding do we do around here?

[BNAndras]

If it breaks the test runner, that’s a defect in how the test runner works. Patch the test runner so it detects when one of those words is used in the problematic way and report it as an error. The error message should say what the problem is (the words being used) and how to fix it (removing those words). You can at the point direct students to the testing docs for more information.

Then, in the testing docs, add an Exercism warning block to the test directions with a few sentences about the issue and why those words are a problem.

[BethanyG]

@axtens

Which brings me all the round to asking myself (and anyone who’s listening) yet again, how much hand-holding do we do around here?

I think the answer is “it depends on the level of attention and disruption”. But if something is outright crashing or borking the test runner … oof … then you have to do something, even if that something is maybe less-than-perfect. An oft-cited strategy is called lie to children.

An example:

Using print() in place of a function return in Python will toss a stack trace (or a confusing error about getting None) under the current tests. So will naming your functions differently than what the tests expect (because the test harness then can’t find and load the functions to test them).

And so? That’s something you need to learn. But we were getting so many complaints and bugs and requests for help that it became problematic. Students were’t reading docs, and they weren’t reading or understanding the stack traces.

So now we have custom error messages for Hello World and the first concept exercise that trim the stack traces and say something like

“We called your function and did not get anything back. Did you use print instead of return??” and

“We couldn’t find the function named ____ to test it. Did you forget to create it or name it something else?”.

But we don’t do that for any other exercises.

Another problematic one is input(), which pauses a program to listen for input supplied on the command line, and causes PyTest to throw an error about how input() is disabled and blah blah blah. We haven’t implemented a custom error message for that one yet because I’ve only seen it once or twice in 5 years - but that’s another place where students get very confused.

Upshot? If the errors or usage are completely blocking students from engaging or progressing, then restricting syntax or customizing errors early on (as shocking as that might be) might be the way to go until they get their bearings.

[axtens]

Thank you. Very much. Indeed. Linting here we come.

[axtens]

So here’s where I demonstrate my ignorance yet again.

# Lint the COBOL for STOP RUN and GOBACK
lint_output=$(grep -P -i '(?<!\*)(STOP RUN|GOBACK)' ${solution_dir}/src/${slug}.cob 2>&1)
if [ $? -eq 0 ]; then
    trimmed_output=$(echo "$lint_output" | sed -E 's/^[[:space:]]+//')
    jq -n --arg output "Your cannot use '${trimmed_output}' in tests." '{version: 1, status: "fail", message: $output}' > ${results_file}
    echo "${slug}: done"
    exit
fi

So if I put that in at line 34 in the run.sh in cobol-test-runner, what does that stuff up?

I expect there’s a simpler way for the left-trim but that’s what Gemini suggested and I only known enough bash to be-ash

[IsaacG]

1. bash or POSIX sh?
2. grep -o might be useful here which would simplify things and remove the need for -P.
3. If you’re using bash, prefer if (( $? == 0 )); then. Or, even nicer (POSIX compatible), if output=$( grep ... ); then.
4. echo something | cmd in bash can be cmd <<< something (herestrings; removes a subprocess).
5. jq can do string formatting for you. jq -n --arg output "${trimmed_output}" '{version: 1, status: "fail", message: "Your cannot use \($output) in tests."}' > ${results_file}
6. ${results_file} should be quoted.

[axtens]

1. If line 1 in cobol-test-runner/bin/run.sh at main · exercism/cobol-test-runner · GitHub is any indication, it’s sh.
2. Thanks the the suggestion about jq.

[Borderite]

Invalidating a solution whenever stop run or goback is present in the code is problematic. It kills valid solutions. Here is an example:

       IDENTIFICATION DIVISION.
       PROGRAM-ID. HAMMING.
          DATA DIVISION.
       WORKING-STORAGE SECTION.
       01 WS-DNA-1 PIC X(32).
       01 WS-DNA-2 PIC X(32).
       01 WS-HAMMING PIC 9(2).
       01 WS-ERROR PIC X(31) VALUE SPACE.
          88 WS-UNEQUAL-STRANDS VALUE "Strands must be of equal length".
       01 WS-LENGTH PIC 99 BINARY VALUE 32.
       01 I PIC 99 BINARY.
       01 NUCLEOBASE-1 PIC X.
       01 NUCLEOBASE-2 PIC X.
       PROCEDURE DIVISION.
       TEST-PARAGRAPH.
           MOVE "GGACTGAAATCTG" TO WS-DNA-1
           DISPLAY 'WS-DNA-1 = "' WS-DNA-1 '"'
           MOVE "GGACTGAAATCTG" TO WS-DNA-2
           DISPLAY 'WS-DNA-2 = "' WS-DNA-2 '"'
           PERFORM HAMMING
           DISPLAY 'WS-HAMMING = ' WS-HAMMING.
       TEST-PARAGRAPH-END.
           STOP RUN.
       HAMMING.
           MOVE 0 TO WS-HAMMING
           PERFORM VARYING I FROM 1 BY 1 UNTIL I > WS-LENGTH
              MOVE WS-DNA-1(I:1) TO NUCLEOBASE-1
              MOVE WS-DNA-2(I:1) TO NUCLEOBASE-2
              IF NUCLEOBASE-1 <> NUCLEOBASE-2 THEN 
                 IF NUCLEOBASE-1 = SPACE OR NUCLEOBASE-2 = SPACE THEN 
                    MOVE 0 TO WS-HAMMING 
                    SET WS-UNEQUAL-STRANDS TO TRUE 
                    EXIT PERFORM
                 END-IF 
                 ADD 1 TO WS-HAMMING 
              END-IF 
           END-PERFORM
           DISPLAY "Hamming Distance is: " WS-HAMMING
           DISPLAY WS-ERROR.
       END PROGRAM HAMMING.

Recall that cobol-check inserts tons of lines into the source code before compilation. So, error messages from the Cobol compiler come with line numbers of the modified code. This is highly inconvenient. The first paragraph TEST-PARAGRAPH in the example code is handy, because it makes it possible to compile the code, run it, and debug it without cobol-check. Without stop run, however, this program does not run correctly.

Also, people can write subprograms and functions. They naturally have goback in them.