charmitro added inline comments.
================ Comment at: clang/test/Frontend/absolute-paths.c:6 +// NORMAL: In file included from [[ROOT_ABSOLUTE]]:4: +// ABSOLUTE: In file included from [[ROOT_ABSOLUTE]]:4: ---------------- tbaeder wrote: > charmitro wrote: > > tbaeder wrote: > > > charmitro wrote: > > > > tbaeder wrote: > > > > > This checks the same thing in both cases, but in the `NORMAL` case, > > > > > it should //not// use the absolute path, shouldn't it? > > > > You're correct. I was constantly testing it from the same path. > > > > > > > > Since the relative path is going to be different depending on where you > > > > running from, would it be wise to accept a regular expression of any > > > > string in the `NORMAL` case? > > > > > > > > For example, > > > > ``` > > > > NORMAL: In file included from {{.*}}: > > > > ``` > > > I wonder if it would make sense to just check for the filename in the > > > `NORMAL` case and then do a `NORMAL-NOT: [[ROOT_ABSOLUTE]]`? > > Yes, that way we are testing if the warning contains the correct > > filename+LOC. > > > > Something like: `// NORMAL: In file included from > > {{.*absolute-paths.c:4}}:`. > > > > But why changefrom `ABSOLUTE:` to `NORMAL-NOT`? > Your regex checks if the path ends with the file name, right? That would be > true in both cases. What else do you propose here? Let me explain why this was the only obvious option for me. In the case of `NORMAL`, it checks for the relative path based on the location you invoke the test commands. Given that, if we, for example, 1. run `llvm-lit` from the project root, the `NORMAL` case expects: ``` In file included from clang/test/Frontend/absolute-paths.c:4: ``` 2. run `llvm-lit` from `$PROJECT_ROOT/clang/test/`, the `NORMAL` case expects: ``` In file included from Frontend/absolute-paths.c:4: ``` It seems unwise to me to hard-code the path based on what the CI excepts in the `NORMAL` scenario, because that way, whoever runs the test cases manually, will experience test failure. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D151833/new/ https://reviews.llvm.org/D151833 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits