<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Wed, Dec 2, 2015 at 9:48 PM, Zachary Turner <span dir="ltr"><<a href="mailto:zturner@google.com" target="_blank">zturner@google.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><br><br><div class="gmail_quote"><span class=""><div dir="ltr">On Wed, Dec 2, 2015 at 9:44 PM Todd Fiala <<a href="mailto:todd.fiala@gmail.com" target="_blank">todd.fiala@gmail.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"> <br></div><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div><span style="line-height:1.5"> and the classname could be dropped (there's only one class per file anyway, so the classname is just wasted space)</span></div></div></blockquote><div><br></div></div></div></div><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div>Part of the reason I included that is I've hit several times where copy and paste errors lead to the same class name, method name or even file name being used for a test.  I think, though, that most of those are addressed by having the path (relative is fine) to the python test file.  I think we can probably get by with classname.methodname (relative test path).  (From your other email, I think you nuke the classname and keep the module name, but I'd probably do the reverse, keeping the class name and getting rid of the module name since it can be derived from the filename).</div></div></div></div></blockquote></span><div>I don't think the filename can be the same anymore, as things will break if two filenames are the same.</div></div></div></blockquote><div><br></div><div>Maybe, but that wasn't my experience as of fairly recently.  When tracking failures sometime within the last month, I tracked something down in a downstream branch with two same-named files that (with the legacy output) made it hard to track down what was actually failing given the limited info of the legacy test summary output.  Maybe that has changed since then, but I'm not aware of anything that would have prohibited that.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div class="gmail_quote"><div>  We could go one step further and enforce this in the part where it scans for all the tests.</div></div></div></blockquote><div><br></div><div>I think I can come up with a valid counterargument to doing that.  I could imagine some python .py files being organized hierarchically, where some of the context of what is being tested clearly comes from the directory structure.</div><div><br></div><div>Something like (I'm making this up):</div><div><br></div><div>lang/c/const/TestConst.py</div><div>lang/c++/const/TestConst.py</div><div><br></div><div>where it seems totally reasonable to me to have things testing const support (in this example) but being very different things for C and C++, being totally uniqued by path rather than the .py file.  I'd prefer not to require something like this to say:</div><div>lang/c/const/TestConstC.py</div><div>lang/c++/const/TestConstC++.py</div><div><br></div><div>as it is redundant (at least via the path hierarchy).</div><div><br></div><div>The other reason I could see avoiding that unique-test-basenames-across-test-suite restriction is that it can become somewhat of an unnecessary burden on downstream branches.  Imagine somebody has a branch and has a test that happens to be running fine, then somebody in <a href="http://llvm.org">llvm.org</a> lldb adds a test with the same name.  Downstream breaks.  We could choose to not care about that, but given that a lot of our tests will revolve around language features accessed/provided by the debugger, and a number of language features pull out of a limited set of feature names (e.g. const above), I could see us sometimes hitting this.  </div><div><br></div><div>Just one take on it.  I'm not particularly wedded to it (I probably would avoid the confusion by doing something exactly like what I said above with regards to tacking on the language to the test name), but I have hit this in similar form across different language tests.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div class="gmail_quote"><div>  If it finds two test files with the same name we could just generate an error.  I think that's a good idea anyway, because if two test files have the same name, then the tests inside must be similar enough to warrant merging them into the same file.</div></div></div></blockquote><div><br></div><div>Maybe, but not in the real cases I saw across different languages.  I think for other areas of the debugger, this isn't an issue.  So maybe language feature tests just have to know to append their language (whether it be C, C++, ObjC, etc.)</div><div> <br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div class="gmail_quote"><div><br></div><div>If no two filenames are the same, and if there's only 1 class per file, then filename + method name should uniquely identify a single test, and so you could omit the class name and show a relative path to the filename.</div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div class="gmail_extra">
</div></div></blockquote></div></div>
</blockquote></div><div class="gmail_extra"><br></div>I think we currently have some tests that have multiple test classes in the test file.  We could certainly verify that in TOT, and we could certainly undo that which seems reasonable.</div><div class="gmail_extra"><br></div><div class="gmail_extra">I'd be interested in what other people think here on restricting test names to be unique across the repo.  I could be convinced either way on allowing two tests with the same name, but I'd probably avoid layering on a restriction if it is entirely artificial and requires longer test names that are otherwise uniqued by path.</div><div class="gmail_extra"><br></div><div class="gmail_extra">Thanks for the feedback!</div><div class="gmail_extra">-- <br><div class="gmail_signature"><div dir="ltr">-Todd</div></div>
</div></div>