Issue metadata
Sign in to add a comment
|
Gerrit inserts Change-Id lines in the middle of a multiline Test: field. |
||||||||||||||||||||
Issue descriptionAffected Version: 2.14.1-1850-ga0eef9c93f What steps will reproduce the problem? 1. Upload a change to PolyGerrit with a multi-line Test: field in the CL description 2. Check the CL description in the Web UI / via `git cl description`. What is the expected output? Change-Id: line is inserted after the Test: line (or before, but not in the middle). What do you see instead? The CL description has become something like this: Bug: <Blah> Test: <First line of test instructions> Change-Id: <Blah> <Second line of test instructions> Please provide any additional information below. Example: https://chromium-review.googlesource.com/c/558541/2
,
Jul 5 2017
,
Jul 5 2017
,
Jul 5 2017
,
Jul 5 2017
There is no such thing as a multiline footer. A footer that looks like Bug: NNN Test: This is a really long line which goes onto the next line Change-Id: Ideadbeef is not a valid footer. "Test: " designators that are longer than one line should be described in prose in the body of the commit message.
,
Jul 10 2017
Issue gerrit:6679 has been merged into this issue.
,
Jul 11 2017
Is there still (or was there ever) any formal usage of "Test:" (or "TEST=") by tooling?
The advice that "if you need more than 1 line, it should be in prose and not prefixed by Test:" would break any formal tooling that looks for "Test:" (e.g., by QA looking to automatically index any manual test instructions). Therefore:
a) If "Test:" is used formally to look for manual tests, this advice is bad and we should be allowed to split a "Test:" over multiple lines. OR,
b) If "Test:" is not used formally at all, then I think we should just drop it as a concept and always use prose ("Test this by verifying that ...").
Either way, it seems weird to use a formal syntax or not, depending on how long the sentence is.
(Note: I've found that there isn't a good understanding of what "Test" means; I always thought it was for specifying manual tests to verify that a change has worked, but I often see people write "Test: Run the test suite" or other things that should be implied by our waterfall policy.)
,
Jul 11 2017
I do not believe there is any automated tooling which consumes "TEST=" or "Test:" tags. There used to be support for parsing it out of the commit message in PRESUBMIT (although that support only worked for single-line tags as well) but no PRESUBMIT files actually called that code, so I removed it a few months ago. We should leave TEST= in the dust as a relic of the past when we didn't have good CI coverage.
,
Jul 12 2017
#8 +1 Can we update any documentation that suggests we do this, and send a PSA to chromium-dev? Who do we have to check with to do this?
,
Aug 7 2017
Issue gerrit:6946 has been merged into this issue.
,
Feb 12 2018
Issue gerrit:8300 has been merged into this issue. |
|||||||||||||||||||||
►
Sign in to add a comment |
|||||||||||||||||||||
Comment 1 by wyatta@google.com
, Jul 5 2017