invalid-module-declaration.rs test regression

[davidtwco]

As part of #53196, compile-fail tests were migrated to ui tests. In doing this, it was discovered that error patterns were not being applied to ui tests (#52531). invalid-module-declaration.rs is a test that regressed due to this and that regression was not noticed.

The error patterns previously on invalid-module-declaration.rs expected the following errors:

// error-pattern: cannot declare a new module at this location
// error-pattern: maybe move this module

In particular, these check for duplicate module declaration errors - which should arise as the auxiliary folder contains modules that have the same name as those being defined within the test code. However, these errors had not been occuring, and a lack of error pattern assertions meant this wasn't noticed.

At some point in a previous commit these did work, as evidenced by old .stderr files for this test. In #53196, the error pattern annotations were removed in order to get it passing so that PR could land.

[RalfJung]

Seems like the error-pattern got originally added with #41501

Cc @GuillaumeGomez @jseyfried

@estebank Is "testsuite" the right label? This is not a bug in the testsuite, this is a regression in diagnostics that went unnoticed because of a (now fixed) bug in the test suite.

[estebank]

@RalfJung fair point. I added it to "testsuite" because I understood that this ticket was taking the place of #52531, not filing a task to fix the regression.

[davidtwco]

I've spent a little bit of time looking into this and I'm not convinced that it is a problem. It seems like the error got changed in #46531 but the error patterns weren't updated (because they weren't being checked and therefore weren't causing an error).

I think what this currently tests is correct and while there exists a branch of the code today that has the error expected by the patterns, I don't think it's possible to make that error happen (at least, I couldn't work out a way to).

[steveklabnik]

It seems this issue is 100% irrelevant now; this test no longer has these declarations, and seems to be working as expected. Closing!