universal-ctags / ctags

A maintained ctags implementation
https://ctags.io
GNU General Public License v2.0
6.57k stars 628 forks source link

Verify unit test inputs are syntactically correct in their own language #1903

Open ahakanbaba opened 6 years ago

ahakanbaba commented 6 years ago

While looking at the unit tests in /ctags/Units/parser-puppetManifest.r I have realized some of the *.pp files have errors in terms of puppet.

The errors I have discovered are the following:

./puppet-scopetest.d/input.pp
Notice: Compiled catalog for <hostname> in environment production in 0.09 seconds
Error: Parameter mode failed on File[/tmp/scopetest]: The file mode specification must be a string, not 'Fixnum' at /home/hbaba/src/universal-ctags/ctags/Units/parser-puppetManifest.r/puppet-scopetest.d/input.pp:5
./unless.d/input.pp
Warning: Unknown variable: 'array'. at /home/hbaba/src/universal-ctags/ctags/Units/parser-puppetManifest.r/unless.d/input.pp:1:8
Error: Evaluation Error: Operator '[]' is not applicable to an Undef Value. at /home/hbaba/src/universal-ctags/ctags/Units/parser-puppetManifest.r/unless.d/input.pp:1:8 on node <hostname>
./puppet-simpleselector.d/input.pp
Error: Evaluation Error: Resource type not found: Yayness at /home/hbaba/src/universal-ctags/ctags/Units/parser-puppetManifest.r/puppet-simpleselector.d/input.pp:30:15 on node <hostname>
./puppet-selectorvalues.d/input.pp
Notice: Compiled catalog for <hostname> in environment production in 0.08 seconds
Error: Parameter mode failed on File[/tmp/selectorvalues1]: The file mode specification must be a string, not 'Fixnum' at /home/hbaba/src/universal-ctags/ctags/Units/parser-puppetManifest.r/puppet-selectorvalues.d/input.pp:43
./puppet-emptyifelse.d/input.pp
Error: Could not parse for environment production: This 'if' statement has no effect. A value was produced and then forgotten (one or more preceding expressions may have the wrong form) at /home/hbaba/src/universal-ctags/ctags/Units/parser-puppetManifest.r/puppet-emptyifelse.d/input.pp:2:1 on node <hostname>
./puppet-collection.d/input.pp
Error: Evaluation Error: Error while evaluating a Function Call, Could not find class ::one for <hostname> at /home/hbaba/src/universal-ctags/ctags/Units/parser-puppetManifest.r/puppet-collection.d/input.pp:10:1 on node <hostname>
./puppet-singleselector.d/input.pp
Notice: Compiled catalog for <hostname> in environment production in 0.10 seconds
Error: Parameter mode failed on File[/tmp/singleselector1]: The file mode specification must be a string, not 'Fixnum' at /home/hbaba/src/universal-ctags/ctags/Units/parser-puppetManifest.r/puppet-singleselector.d/input.pp:20
./puppet-append.d/input.pp
Error: Could not parse for environment production: The operator '+=' is no longer supported. See http://links.puppetlabs.com/remove-plus-equals at /home/hbaba/src/universal-ctags/ctags/Units/parser-puppetManifest.r/puppet-append.d/input.pp:4:10 on node <hostname>
./node.d/input.pp
Error: Could not find node statement with name 'default' or '<hostname>, hbaba.dev.box, hbaba.dev, hbaba' on node <hostname>
./puppet-collection_within_virtual_definitions.d/input.pp
Error: Could not parse for environment production: The parameter $name redefines a built in parameter in the 'define' expression at /home/hbaba/src/universal-ctags/ctags/Units/parser-puppetManifest.r/puppet-collection_within_virtual_definitions.d/input.pp:1:13 on node <hostname>

Maybe running an acceptance test on these unit test input files is the right thing to do. Overtime these files change via manual edits and we want to make sure they are still syntactically correct.

For puppet something like the following can be executed to verify correct syntax.

parser-puppetManifest.r (master)]$ find . -name "*.pp" | xargs -n 1 -I@ bash -c " echo @ &&  puppet apply --noop @"
b4n commented 6 years ago

Agreed, for all but expected-failures we should most probably have valid test files.

masatake commented 6 years ago

I have to use the usual phrase: pull requests are welcome.

masatake commented 6 years ago

We should put the command line for verifying the syntax to somewhere in our source tree. @ahakanbaba, could you write it to ~/var/ctags/Units/parser-puppetManifest.r/README ? This issue is parser-independent. So, we should provide a common tool and mechanism to verify the syntactical correctness of the test inputs. The tool must ignore "expected-failures" inputs. The temporary solution is just putting the README file.

ahakanbaba commented 6 years ago

Agreed, for all but expected-failures we should most probably have valid test files.

Thank for your comments @b4n . What do you mean exactly by "expected-failures"?

While creating this issue, I was under the impression that ctags unit tests should always use valid input language. But upon further thought, that may not be true. Thinking about the ctags use-cases, the user can run ctags anytime on any file. To give an example, even my puppetManifest file has a puppet syntax error, a ctags run on it should not get stuck at 100% cpu indefinitely. And ctags repo may have a test for that. That test would use a puppet file that is syntactically incorrect.

Given that requirement, not all test input files have to be syntactically correct in the original languages. But I do not know whether those type of tests reside in Units/* directory. (I am not familiar with ctags repo yet)

What are the "expected-failures" test cases inside the Units library ?

masatake commented 6 years ago

@ahakanbaba, what you wrote is correct. The Units directory holds both types of test inputs. I didn't define the strict rule for distinguishing the types. However, in the most of all cases, the following rule can be applied: if a .d directory has an expected.tags file, associated input file should be syntactically correct.