[PATCH] D142441: [Docs] Add best practices for regression tests

[Sanjay Patel via Phabricator via llvm-commits]

spatel added a comment.

LGTM - see inline for minor edits.

Nice! I was wondering if we could adapt/copy:
https://developers.redhat.com/articles/2022/12/20/how-contribute-llvm#
...to the LLVM doc pages directly. This is a great start.



================
Comment at: llvm/docs/TestingGuide.rst:321-325
+- Unless the test is for a crash or assertion failure, first commit the test
+  with baseline check lines, i.e. either showing a miscompile or missing
+  optimization. Such test additions can be committed without review. Your
+  actual patch will then only contain a check line diff, which makes it easier
+  to see the effect of your patch.
----------------
```
- If the test does not crash, assert, or infinite loop, commit the test
with baseline check-lines. That is, the test will show a miscompile 
or missing optimization. Add a "TODO" or "FIXME" comment to
indicate that something is expected to change in a test. 

- A follow-up patch with code changes to the compiler will then 
show check-line differences to the tests, so it is easier to see the 
effect of the patch. Remove TODO/FIXME comments added in the
previous step if a problem is solved.

- Baseline tests (no-functional-change or NFC patch) may be pushed 
to main without pre-commit review if you have commit access 
(link to getting started page?).
```


================
Comment at: llvm/docs/TestingGuide.rst:328-329
+  section) whenever feasible.
+- Especially for regression tests, include a comment on what is being tested,
+  and include a reference to any relevant issue on the bug tracker.
+- Avoid undefined behavior unless necessary. For example, do not use patterns
----------------
No need to repeat clause about regression tests - this section only applies to those.

```
- Include comments about what is tested/expected in a particular test. If there are
relevant issues in the bug tracker add references to those bug reports (for example, 
"See PR999 for more details").
```


================
Comment at: llvm/docs/TestingGuide.rst:330-332
+- Avoid undefined behavior unless necessary. For example, do not use patterns
+  like ``br i1 undef``, which are likely to break as a result of future
+  optimizations.
----------------
```
 - Avoid undefined behavior or values unless necessary.
```


================
Comment at: llvm/docs/TestingGuide.rst:338-339
+  running the test through ``opt -S -passes=instnamer``.
+- Try to give SSA variables meaningful names, and avoid retaining complex names
+  generated by the optimization pipeline (such as ``%foo.0.0.0.0.0.0``).
+
----------------
Remove "SSA" - this is true in general for source-level tests too?


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D142441/new/

https://reviews.llvm.org/D142441