[flang-dev] RFC: Port test_modfile.sh tests in Flang to use FileCheck

Wed Jul 8 13:34:57 PDT 2020

On 7/7/20 1:16 PM, Timothy Keith via flang-dev wrote:
> My opinion is that the new form is less readable, is less maintainable, and produces less comprehensible error messages on failures.
>
> Readability: The essentials for this kind of test is the Fortran source and the expected name and contents of the .mod file that is produced. The rest is boilerplate. In the original form that consists of one RUN command with the expected results in comments. In the new form it is three RUN commands plus one per module in the test plus the NO_MODFILES line; expected results are prefixed with a variety of strings.
>
> Maintainability: To add a module to a test, for example, requires adding the source and expected results, adding another RUN line, and changing the NO_MODFILES line. If something has to change in how the tests are run (e.g. we probably need to delete the temp directory at the start of the test), that has to be changed in all 36 tests rather than once in test_modfile.sh.
>
> Quality of messages on failure: I'll add some examples below.
>
> This is a heavy price to pay for "making more clear exactly what the test is doing without needing to also understand how test_modfile.sh works". Moreover, I don't agree it is more clear. The original form encapsulates the boilerplate into test_modfile.sh which in my opinion makes the tests clearer and simpler.
>
> Tim

I agree with Tim, the current errors are clearer and there's less 
boilerplate. A few thoughts:

  0. I don't see a need to use FileCheck for all tests; clang has a 
-verify mode, etc. and if we have a set of tests with specialized needs 
I don't see any reason not to have a specialized checker for them. That 
said...

  1. Does the current script run on Windows? If we keep it, we probably 
need to rewrite it in Python.

  2. The current script doesn't seem to have any comments on top 
documenting how it works. We should have documentation.

  3. My preference would be for the `!Expect: m.mod` lines, etc. to have 
directives in all caps like FileCheck - that makes them stand out more 
and, in my opinion, easier to read.

  4. I don't find FileCheck's output in the first case below all that 
bad -- it's not as succinct as diff's output, but it's okay. We might 
have a wrapper that calls FileCheck instead of calling diff.

As time allows, we can also discuss this on Monday's call.

  -Hal

>
>
> Examples of test failure messages:
>
> 1. .mod file contents are different from what was expected
>
> old form:
> 	Module file m.mod differs from expected:
> 	@@ -1,11 +1,11 @@
> 	 module m
> 	 integer(4)::i
> 	 integer(4),private::j
> 	 type::t
> 	-integer(4)::ii
> 	+integer(4)::i
> 	 integer(4),private::j
> 	 end type
> 	 type,private::u
> 	 end type
> 	 type(t)::x
> 	 end
>
> new form:
> 	test/Semantics/modfile01.f90:33:16: error: MOD-M1-NEXT: expected string not found in input
> 	! MOD-M1-NEXT: integer(4)::ii
> 	               ^
> 	build/Release-clang-9.0.0-ccache/test/Semantics/Output/modfile01.f90.tmp.dir/m1.mod:6:1: note: scanning from here
> 	integer(4)::i
> 	^
> 	
> 	Input file: build/Release-clang-9.0.0-ccache/test/Semantics/Output/modfile01.f90.tmp.dir/m1.mod
> 	Check file: test/Semantics/modfile01.f90
> 	
> 	-dump-input=help describes the format of the following dump.
> 	
> 	Full input was:
> 	<<<<<<
> 	         1: !mod$ v1 sum:cfaed627f0a11097
> 	         2: module m1
> 	         3: integer(4)::i
> 	         4: integer(4),private::j
> 	         5: type::t
> 	         6: integer(4)::i
> 	next:33     X~~~~~~~~~~~~ error: no match found
> 	         7: integer(4),private::j
> 	next:33     ~~~~~~~~~~~~~~~~~~~~~
> 	         8: end type
> 	next:33     ~~~~~~~~
> 	         9: type,private::u
> 	next:33     ~~~~~~~~~~~~~~~
> 	        10: end type
> 	next:33     ~~~~~~~~
> 	        11: type(t)::x
> 	next:33     ~~~~~~~~~~
> 	        12: end
> 	next:33     ~~~
> 	>>>>>>
>
> 2. Extra or missing .mod file
>
> old form:
> 	Unexpected .mod files produced: m5.mod
> or:
> 	Compilation did not produce expected mod file: m5.mod
>
> new form:
> 	test/Semantics/modfile01.f90:6:16: error: NO_MODFILES: expected string not found in input
> 	! NO_MODFILES: 4
> 	               ^
> 	<stdin>:1:2: note: scanning from here
> 	 5
> 	 ^
> 	
> 	Input file: <stdin>
> 	Check file: /Users/tsk/llvm/llvm1/flang/test/Semantics/modfile01.f90
> 	
> 	-dump-input=help describes the format of the following dump.
> 	
> 	Full input was:
> 	<<<<<<
> 	         1:  5
> 	check:6      X error: no match found
> 	>>>>>>
>
> On 7/7/20, 9:27 AM, "flang-dev on behalf of Richard Barton via flang-dev" <flang-dev-bounces at lists.llvm.org on behalf of flang-dev at lists.llvm.org> wrote:
>
>      External email: Use caution opening links or attachments
>
>
>      Hi flang-dev
>
>      I'm looking at porting the Flang regression tests over to using standard LLVM utilities like FileCheck.
>
>      The modfile tests are a bunch of tests in Semantics that define modules and have check lines saying which module files are expected to be created and what they are expected to contain. The test_modfile.sh script runs Flang on the sources to generate said module files then checks that all expected module files have the correct contents and that no additional, unexpected module files are created.
>
>      The ported tests will exec Flang in the same way to create modfiles. We then use FileCheck on each module file in turn by name to ensure the correct name is generated and check the contents in the same way using CHECK-NEXT to ensure no additional information is added. The ported tests use the -module option to put the module files in a temporary location to make sure the tests cannot spuriously pass due to a dirty build directory. To check that no extra modfiles are generated, the tests use ls on the temporary location and ensure the right number of files are present.
>
>      Here is an example port of modfile01: https://reviews.llvm.org/D83320
>
>      The ported tests are more verbose than the previous approach using test_modfile.sh, at the expense of making more clear exactly what the test is doing without needing to also understand how test_modfile.sh works. I think the ported tests are preferable to the original on this principal of least surprise.
>
>      If folks are happy with this porting approach then I will do the rest of the tests and re-submit as a single patch.
>
>      Note: at present, lit has a bug when printing out UTF characters to a non-UTF shell if run with python 2. Flang's modfiles contain a short UTF BOM at the start of the file so the lit issue would be hit were these tests to fail in such an environment with -v or -a enabled. I have submitted a fix for this issue here: https://reviews.llvm.org/D82754 and I will not push the ports until that issue is fixed.
>
>      Thanks
>      Rich
>
>      Richard Barton
>      Principal Engineer - Arm Compiler for Linux
>      Arm Manchester
>
>      _______________________________________________
>      flang-dev mailing list
>      flang-dev at lists.llvm.org
>      https://lists.llvm.org/cgi-bin/mailman/listinfo/flang-dev
>
> _______________________________________________
> flang-dev mailing list
> flang-dev at lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/flang-dev

-- 
Hal Finkel
Lead, Compiler Technology and Programming Languages
Leadership Computing Facility
Argonne National Laboratory