Page MenuHomePhabricator

[analyzer] [NFC] [tests] Move plist-based diagnostics tests to separate files, use diff instead of a FileCheck
ClosedPublic

Authored by george.karpenkov on Aug 9 2018, 6:22 PM.

Details

Summary

Some of the analyzer tests check the exact plist output, in order to verify that the diagnostics produced is correct.
Current testing setup has many issues:

  • plist output clobbers tests, making them harder to read
  • it is impossible to debug test failures given error messages from FileCheck. The only recourse is manually creating the files and using the diff
  • again, it is impossible to update the tests given the error message: the only process is a tedious manual one, going from a separate plist file to CHECK directives.

This patch offers a much better approach of using "diff" directly in place of FileCheck, and moving tests to separate files.

Generated using the following script:

#!/usr/bin/env python
import os
import glob
import re
import subprocess

diagnostics_key = "// CHECK:  <key>diagnostics</key>"

def process_file(f, data):
    idx = data.index(diagnostics_key)
    plist_out_f = 'ExpectedOutputs/plists/%s.plist' % f
    plist_out_folder = os.path.join('ExpectedOutputs/plists/', os.path.dirname(f))
    plist_data = data[idx:]
    plist_data = plist_data.replace('// CHECK: ', '')
    plist_data = plist_data.replace('// CHECK-NEXT: ', '')
    plist_data += "</dict>\n</plist>\n"
    data = data[:idx]

    ptn = re.compile("FileCheck --?input-file(=| )(%t|%t\.plist) %s")

    if not ptn.findall(data):
        print "none found =/ skipping..."
        return

    data = ptn.sub(lambda m: "tail -n +11 %s | diff -u -w - %%S/../%s" % (m.group(2), plist_out_f), data)

    with open(f, 'w') as out_f:
        out_f.write(data)

    subprocess.check_call(["mkdir", "-p", plist_out_folder])
    with open(plist_out_f, 'w') as out_f:
        out_f.write(plist_data)


def main():
    files = glob.glob("**/*.*")
    for f in files:
        with open(f) as f_handler:
            data = f_handler.read()
        if diagnostics_key in data:
            print "Converting %s" %f
            process_file(f, data)

if __name__ == "__main__":
    main()

Diff Detail

Repository
rC Clang

Event Timeline

NoQ added a comment.Aug 9 2018, 6:31 PM

I very much approve this and hope it actually runs on all platforms on which LLVM is tested.

clang/test/Analysis/ExpectedOutputs/plists/convert.py.plist
3–36 ↗(On Diff #160044)

Is this expected to be here?

clang/test/Analysis/ExpectedOutputs/plists/cstring-plist.c.plist
9 ↗(On Diff #160044)

Should there be a newline at end of file?

clang/test/Analysis/ExpectedOutputs/plists/objc-radar17039661.m.plist
1276 ↗(On Diff #160044)

Should there be a newline at end of file?

clang/test/Analysis/ExpectedOutputs/plists/unix-fns.c.plist
2834 ↗(On Diff #160044)

Should there be a newline at end of file?

clang/test/Analysis/ExpectedOutputs/plists/yaccignore.c.plist
5 ↗(On Diff #160044)

Should there be a newline at end of file?

This is awesome! I'm working on a number of potentially plist changing projects, so I feel the pain of the FileCheck related issues.

george.karpenkov marked 5 inline comments as done.

hope it actually runs on all platforms

Both Clang and LLVM have tests which use "diff" directly, so I think it should work. In the worst case, it can be crudely emulated with a script.

NoQ accepted this revision.Aug 10 2018, 2:29 PM
This revision is now accepted and ready to land.Aug 10 2018, 2:29 PM
This revision was automatically updated to reflect the committed changes.