Clarify/Improve instructions related to testing when contributing a pull request

[karrtikr]

Originally posted by @flying-sheep in #20842 (comment)

Hi, I’m unsure what to do regarding testing. I guess “just add a unit test” is probably correct, but I didn’t find clear instructions. Maybe this could be improved?

1. CONTRIBUTING.md links to the wiki main page. It doesn’t have in-text links to anything about contributing or testing, but there’s a sidebar with all wiki pages. Maybe it should mention the right page to go to when trying to contribute?

2. The wiki page Submitting a pull request doesn’t mention what tests to add. Maybe it should say “Go to the Testing page and add one or more tests that best fit your change”?

3. The wiki page Testing mentions a lot of options in great detail, but not what exactly to do when creating a PR. Maybe it should do that?

4. The check output is also not very clear: This “Check for changed files” run says

   TypeScript code was edited without also editing a "src//*.test.ts\nsrc//*.testvirtualenvs.ts\n.github/test_plan.md" file; see the Testing page in our wiki on testing guidelines (the "skip tests" label can be used to pass this check)

   The file pattern in there implies I should ask a test plan, but from context I assume this means that I should add a unit test (or in case my change wouldn’t be testable, add a test plan document), is that right? Maybe the file pattern should be removed or explained

[karrtikr]

The file pattern in there implies I should ask a test plan,

It means there were no patterns found related to:

• src//*.test.ts
• src//*.testvirtualenvs.ts
• .github/test_plan.md

Meaning that no tests were added. Now it may happen that no tests are needed for the change, in which case skip tests label should be used.

The wiki page Testing mentions a lot of options in great detail, but not what exactly to do when creating a PR. Maybe it should do that?

Can you clarify what're you looking for exactly? Currently the testing page basically points to the test files (*.unit.test.ts etc) you can look for inspiration, and it details the mechanism you can use to run/debug a test.

[flying-sheep]

Hi! That makes sense! I think I needed to read 4 different documents in addition to the error message though, and I feel like it could be modified so it points people in the right direction from the start.

Maybe change the error message so it contains the description with a link first and the failing file pattern last or so? And that link points to a wiki page that directly says “If you want to contribute, you need to ceate at least one test. The following kinds of tests exist: …”

Just a suggestion, maybe there’s a better way!

[karrtikr]

Sounds reasonable to me 👍 Adding appropriate labels so we can take care of it.

[eleanorjboyd]

took a look at this and still feel it is a very relevant point- finding the steps are hard and take a lot of clicks. Assigning myself to get it on my list of items