Adapt creation of AUTHORS.sorted to changes in format of AUTHORS

[rartino]

The recent changes in the format of AUTHORS have broken the automatic auditing of the alphabetical order in AUTHORS. This PR fixes that problem.

Would be great to get this in for v1.1.

[CasperWA]

How does a content change in the AUTHORS_TAG variable fix the automatic auditing? Am I missing something? 😅

[rartino]

How does a content change in the AUTHORS_TAG variable fix the automatic auditing? Am I missing something?

If nothing matches the AUTHORS_TAG, then the whole file is counted as "header" and nothing gets sorted in AUTHORS.sorted, which makes `make audit_authors' always pass.

[ml-evs]

Thanks for this @rartino, wasn't aware we had broken it.

Not for this PR, but did we ever discuss about adding workshop 2021 attendees to the list? I guess we can argue here that for 1.1, the 2021 workshop didn't really add anything.

[CasperWA]

If nothing matches the AUTHORS_TAG, then the whole file is counted as "header" and nothing gets sorted in AUTHORS.sorted, which makes `make audit_authors' always pass.

Okay - this seems a bit beyond my basic knowledge of this then. But how can I then see that this issue exists from the tests? I guess I can't if it passes in both cases - is there a way this could be made more transparent though? I mean, if I compare the "Tests / Audit the AUTHORS list" with a similar test run that doesn't have this change I see absolutely no difference - which I would also expect if it passes in both cases, but I'd like to be able to see a difference to confirm this now works (and maybe stops at some point). Would it be possible to cat some file contents or similar to be able to also manually confirm what the tests proclaim? :)

[rartino]

if I compare the "Tests / Audit the AUTHORS list" with a similar test run that doesn't have this change I see absolutely no difference - which I would also expect if it passes in both cases, but I'd like to be able to see a difference to confirm this now works (and maybe stops at some point). Would it be possible to cat some file contents or similar to be able to also manually confirm what the tests proclaim? :)

Swap two names in the AUTHORS list, run 'make audit_authors' and note the difference?

It isn't a super unusual thing that a change breaks a test so that it no longer tests the intended thing. Usually one can try to program defensively so the test more easily returns a false positive than a false negative, but for something as non-critical as ordering the AUTHOR list alphabetically I wouldn't overdo things. But if you are greatly concerned, I suppose we can add code to the test to verify that the sorted part of the file has non-zero length.

[CasperWA]

It isn't a super unusual thing that a change breaks a test so that it no longer tests the intended thing. Usually one can try to program defensively so the test more easily returns a false positive than a false negative, but for something as non-critical as ordering the AUTHOR list alphabetically I wouldn't overdo things. But if you are greatly concerned, I suppose we can add code to the test to verify that the sorted part of the file has non-zero length.

When you put it like that, I agree that putting more work into this seems counter-productive, unless it is an extremely easy check to add. Otherwise, yeah. all good for me.

Just questioning the usefulness of the test here if it didn't catch this. Also after knowing that tests cannot reasonably take into account all edge cases.

[rartino]

unless it is an extremely easy check to add. Otherwise, yeah. all good for me.

I propose merging the PR as it is and possibly improve the test in a future PR.

Just questioning the usefulness of the test here if it didn't catch this. Also after knowing that tests cannot reasonably take into account all edge cases.

As long as the line before the list of alphabetically ordered AUTHORS remain intact (something we've changed maybe 1-2 times during the lifetime of this project), the test fixes errors that we frequently do. That seems useful to me.