Subject: bug#66782: 29.1; ERT tests report test redefined depending on loading sequence

From: Xiyue Deng <[email-masked]>
Date: Sat, 28 Oct 2023 14:19:40 -0700

Mattias Engdegård <[email-masked]> writes:

> 28 okt. 2023 kl. 14.03 skrev Eli Zaretskii <[email-masked]>:
>
>> We could reasonably expect from the author of a single .el test file
>> to make sure the tests in that file don't interfere with one another
>> and make the necessary cleanups for that purpose.  But it is much
>> harder to expect the same from several disparate test files, which can
>> quite frequently be written and maintained by different people.
>
> I agree, and I have found this to be far from uncommon in practice.
>
> Treating redefined test cases as errors found and still finds serious testing
> bugs, so we definitely want to keep the check as it is or in some equivalent
> form (a warning wouldn't be nearly as good).
>

I also agree, and I would definitely keep the current check.  At the
very least, duplicated test name makes identifying failed tests harder.

> And I also agree that the way this test runner explicitly loads .el files
> without any concern for whether they logically represent a single test
> collection or are just sub-modules for use in other tests, is quite
> questionable.
>

Exactly.

> I may be missing the point entirely, but what about using a file name convention
> like Emacs, where only *-tests.el are considered to be actual tests?
>

Which is why I'm asking for upstream stance on whether it will consider
requiring other test modules as well-formed.  To clarify one of my
original proposals: if it makes no sense for a `ert-deftest' to depend
on other `ert-deftest', I would say there is no reason for such a
dependency, and people should move reusable components to a helper
module instead.  And of course a better error message such as forbidding
such requires would be better.

Wdyt?

-- 
Xiyue Deng