@isabelle-dr changing this to a draft to avoid accidental merging - realizing we could use a few more test cases and document good/bad data examples

@bdferris-v2 Any thoughts on using parameterized tests where they make sense? For this where there are many ways the short_name can be contained in the long_name, it seems like a perfect candidate. Plus some way off day in the future you might be able to use tests params as sample good/bad data in the generated documentation :)

I see this library which looks useful, but wondering if you had any other recommendations or concerns with doing this.

@isabelle-dr @bdferris-v2 I've made some significant changes. Documenting the rule made me realize it was missing some use cases and that it may not be perfectly clear and/or covering all potential issues.

So far, I've updated to account for short/long are equal (case insensitive), long cannot start with short and be followed by a space, or disallowed character (, -. I added ) and am wondering if : or other characters should be added?

Here are my current test cases for bad data:

           // short, long
            {"L1", "L1 Long Name"},
            {"L1", "L1-Long Name"},
            {"L1", "L1- Long Name"},
            {"L1", "L1 - Long Name"},
            {"L1", "L1)Long Name"},
            {"L1", "L1) Long Name"},
            {"L1", "L1 ) Long Name"},
            {"L1", "L1(Long Name)"},
            {"L1", "L1 (Long Name)"},
            {"L1", "L1( Long Name)"},

Please let me know if any should be removed or added.

@bdferris-v2 Made those fixes, thanks.

One last small change before we merge :)

@isabelle-dr because the acceptance test workflow only has permission to post the comment with the report summary if the PR is coming from a branch of the gtfs-validator repo itself, as opposed to a fork of the repo, as is the case with this PR (and many coming from external contributors). This is discussed in more detail here. Unfortunately, I've not seen a good way around this.

@bdferris-v2 Any thoughts on using parameterized tests where they make sense? For this where there are many ways the short_name can be contained in the long_name, it seems like a perfect candidate. Plus some way off day in the future you might be able to use tests params as sample good/bad data in the generated documentation :)

I see this library which looks useful, but wondering if you had any other recommendations or concerns with doing this.

That's a good call. Definitely the bad list test case is a great example for parameterizing a test. We can have this "feature" for free if we decide to upgrade the JUnit version. JUnit 5 Parameterized Tests
Let's keep the topic alive and have the conversion out of scope of this PR.

@isabelle-dr because the acceptance test workflow only has permission to post the comment with the report summary if the PR is coming from a branch of the gtfs-validator repo itself, as opposed to a fork of the repo, as is the case with this PR (and many coming from external contributors). This is discussed in more detail here. Unfortunately, I've not seen a good way around this.

@isabelle-dr @bdferris-v2 This was created before I had access to the main repo. Should I close this PR and open a new one from a local branch?

Two things. The acceptance test still ran and you can access the logs in the Checks tab to see the result:

✅ Rule acceptance tests passed.
New Errors: 0 out of 1419 datasets (~0%) are invalid due to code change, which is less than the provided threshold of 1%.
Dropped Errors: 0 out of 1419 datasets (~0%) are invalid due to code change, which is less than the provided threshold of 1%.
0 out of 1419 sources (~0 %) are corrupted.
Commit: 4a956ff
Download the full acceptance test report here (report will disappear after 90 days).
✅ Rule acceptance tests passed.

That said, per #1349, the acceptance test only tracks changes to ERROR notices, not warnings. As this PR only changes a warning notice, it wouldn't show up as a change in the acceptance test. That's something I'd like to fix.

@briandonahue good to merge?

@briandonahue good to merge?

@isabelle-dr Yes! 😄