ilovepi wrote:

> @ilovepi
> 
> > There's a large number of tests. Many of these look like they're close to 
> > an existing test ... in these cases is it possible to just use the existing 
> > test file and add checks under a prefix?
> 
> I originally recommended that @anthonyhatran write new tests so this is on 
> me. We'll take a look and see if adding to existing tests is worth it.
> 

If those were your instructions I'm fine with it. I think either the 
programmer's manual or the testing guide suggest limiting the number of files. 
Since I  think many of these are copies of existing tests, I think it could 
make sense to group the checks, but I'm totally OK with us organizing these 
differently (e.g. I don't have strong opinions here :) )

> FWIW In general I'm actually not a fan of adding to existing tests to avoid 
> tests becoming large, unwieldy, and annoying to debug when they fail. I 
> prefer individual tests to test as few things as possible. As with many 
> things, context is really important so we'll take a look.

Agreed, but I don't mind *related* things being tested together. I'm perfectly 
OK with the status quo, especially since this was a directive from another 
reviewer.


https://github.com/llvm/llvm-project/pull/145967
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to