Do Github CI tests pass even if stdout/stderr from p4c does not match committed expected output files?

[jafingerhut]

There is some evidence I have seen that when one makes changes to p4c that should cause stdout/stderr to change, in a way that the new stdout/stderr differs from expected output files committed in directories with directory names matching the pattern testdata/*_output, the CI test still passes, despite that mismatch.

This seems like a bug in the testing code somewhere, that would be good to fix.

[fruffy]

See #5317. I investigated this and it looks like this is working as intended. We should consider simplifying the way we check stdout/stderr.

The stdout and stderr also need to be in the right folder iirc. Sometimes we have matches in the outputs folder, sometimes in the err folder.

[jafingerhut]

If it is working as intended, then I don't see how a recent commit on this PR is passing, unless the last commit does not change the warning messages as desired: #5342 (comment)

[ChrisDodd]

There's a feature that if the test P4 program contains the string P4TEST_IGNORE_STDERR (generally in a comment), then mismatches in stderr are ignored and won't cause the test to fail. That is what was happening in the specific case of #5317 (which is why it was closed as "working as intended")

If there are tests that don't have this string that pass anyways, it would be a bug, but there is nothing in the test OUTPUT that indicates the string was found and the mismatch is being ignored. It just prints the same diff, and then exits with a success exit code (0) rather than a failure. Perhaps the script should be enhanced to print a message like "stderr differences being ignored due to P4TEST_IGNORE_STDERR" when this happens.

[jafingerhut]

It sounds like the p4c PR that led me to think that there might be a problem here, I was mistaken about what was happening. What was actually happening was that the p4c code and the test cases were both being updated near the same time, but I failed to notice one of the commits that was updating the expected output files. Thus those CI tests were passing, and should have been passing.

Sorry for the noise, if that is all it was, and glad to see some improvements were made in this area recently.

Chris's idea in a comment above about "stderr differences being ignored due to P4TEST_IGNORE_STDERR" sound like a reasonable idea for explicitness, but unless someone wants to implement that change, it seems reasonable to close this issue.