[obj2yaml][yaml2obj]Locate all tests, including .yaml and .test

[jhenderson]

Summary

A number of the obj2yaml tests end in .yaml, but .yaml is not a default file type picked up by lit, so these tests weren't being run when running the testsuite as a whole (they could be run explicitly still). This change adds a lit local config file to specify the known file types for obj2yaml tests (.yaml and .test). Additionally, it fixes the yaml2obj config file to allow both .test and .yaml suffixed tests (previously, the two tests ending in '.test' were not being run).

[grimar]

Something seems wrong here. Phab shows you modified lit.local.cfg,
but the path starts from "C:/llvm" and also I do not have lit.local.cfg file in my llvm\test\tools\obj2yaml folder.

Also, when I run ninja check-llvm (under ubuntu), it actually runs the tests from llvm\test\tools\obj2yaml for me.
Could you point me how to use it?

[grimar]

i.e. for example, I have verdef-section.yaml from D58437 placed in tools/yaml2obj and no code applied.
And check-llvm fails for me as expected:

umb@umb-virtual-machine:~/LLVM/build$ ninja check-llvm
[1/99] Generating VCSRevision.h
-- Found Subversion: /usr/bin/svn (found version "1.9.3") 
[22/23] Running the LLVM regression tests
FAIL: LLVM :: tools/yaml2obj/verdef-section.yaml (29882 of 29889)
******************** TEST 'LLVM :: tools/yaml2obj/verdef-section.yaml' FAILED ********************
Script:
--
: 'RUN: at line 1';   /home/umb/LLVM/build/bin/yaml2obj /home/umb/LLVM/llvm/test/tools/yaml2obj/verdef-section.yaml -o /home/umb/LLVM/build/test/tools/yaml2obj/Output/verdef-section.yaml.tmp
: 'RUN: at line 2';   /home/umb/LLVM/build/bin/llvm-readelf -V /home/umb/LLVM/build/test/tools/yaml2obj/Output/verdef-section.yaml.tmp | /home/umb/LLVM/build/bin/FileCheck /home/umb/LLVM/llvm/test/tools/yaml2obj/verdef-section.yaml
--
Exit Code: 1

Command Output (stderr):
--
YAML:47:7: error: unknown key 'Entries'
      - Version:         1
      ^
yaml2obj: Failed to parse YAML file!

--

********************
Testing Time: 765.72s
********************
Failing Tests (1):
    LLVM :: tools/yaml2obj/verdef-section.yaml

[grimar]

Oh, I just noticed. My test is for tools/yaml2obj, but yours case is about tools/obj2yaml.

So seems this patch tried to add a lit.local.cfg to tools/obj2yaml folder.
That change LGTM (not sure what is wrong with the patch though as it shows that file was modified, and not added).

[jhenderson]

Not quite sure what was/is going on. I ran my diff script incorrectly before, hence the "C:" bit. I don't know why this file is showing up as modified rather than added. I've uploaded a diff with the path cleaned up.

I see the issue when I explicitly run lit on the obj2yaml test folder (i.e. running the llvm-lit.py script on Windows). I haven't tried check-llvm etc, so it's possible that uses a different mechanism to find the tests, but are you sure it's running the tests under test/tools/obj2yaml ending in '.yaml'? Either way the basic test/lit.cfg.py clearly listst the default config.suffixes as config.suffixes = ['.ll', '.c', '.cxx', '.test', '.txt', '.s', '.mir'].

[jhenderson]

Oops, didn't see your later responses. Thanks!

[jhenderson]

Update config files.

[jhenderson]

Turns out that the yaml2obj config file was also broken, and I just traded one bad thing for another. The suffixes list was being completely overwritten by a list containing only '.yaml', so only tests ending in .yaml were being run. This meant that we replaced 3 running tests with 5, when it should in fact be 8 in obj2yaml. I've fixed the yaml2obj config here too, increasing the number of tests from 27 to 29.

@grimar, please could you review again!

[grimar]

This looks fine, very nice catch, btw!

Have a question though.
I looked around in llvm and it seems the common practice is to override the lists.
Maybe we should just do config.suffixes = ['.test', '.yaml'] too?
I would not expect to see other suffixes in test/tools/yaml2obj and test/tools/obj2yaml.
Moreover, when I think about other suffixes it seems it might happen only because of a mistake to me.

[jhenderson]

Yes, you're probably right. I'll do that instead.

[jhenderson]

Explicitly only accept .test and .yaml tests.

[grimar]

LGTM

test/tools/obj2yaml/lit.local.cfg

+config.suffixes.add('.yaml')

test/tools/yaml2obj/lit.local.cfg

-config.suffixes = ['.yaml']
+config.suffixes.add('.yaml')