Mqe 1734 Add base + merge files to Test Description #468
Conversation
| { | ||
| $title = ''; | ||
| foreach ($fileNames as $fileName) { | ||
| if (!empty($fileName && realpath($fileName) !== false)) { |
There was a problem hiding this comment.
Line should read
if (!empty($fileName) && realpath($fileName) !== false) {
Note empty() function closing parenthesis
There was a problem hiding this comment.
Fixed. I wonder how it even worked?
| DIRECTORY_SEPARATOR | ||
| ); | ||
| if (!empty($relativeFileName)) { | ||
| if (empty($title)) { |
There was a problem hiding this comment.
You can get rid of this conditional by setting $title = '<br><br>Test Files:<br>' on line 184 and returning return $description .= $title instead.
I understand that this would mean if no filenames are passed in that you'd have a partial description, but I think that's okay since we want to be able to easily tell processing of it went wrong.
There was a problem hiding this comment.
Another side effect of this is that it will break unit tests.
| @@ -118,6 +118,21 @@ public function extractTestData($testData) | |||
| //Override features with module name if present, populates it otherwise | |||
| $testAnnotations["features"] = [$module]; | |||
|
|
|||
| // Append test file names in description annotation | |||
| if (!empty($fileNames)) { | |||
| if (isset($testAnnotations["description"][0])) { | |||
There was a problem hiding this comment.
Need to check if description array is set at all and set it. You can also simplify these conditionals:
if (!empty($fileNames)) {
if (!isset($testAnnotations["description"])) {
$testAnnotations["description"] = [];
}
$description = $testAnnotations["description"][0] ?? '';
$testAnnotations["description"][0] = $this->appendFileNamesInDescriptionAnnotation($description, $fileNames);
}
There was a problem hiding this comment.
My code just set the array value directly regardless of description array is set or not, but your code looks more strait forward. I will double check and change it if it works.
| ); | ||
| if (!empty($relativeFileName)) { | ||
| if (empty($title)) { | ||
| $title .= '<br><br>Test Files:<br>'; |
There was a problem hiding this comment.
Asked around for opinions, we should bold the <b>Test Files:</b> for better readability.
There was a problem hiding this comment.
It's obvious that bold is better. I will just go ahead change it
|
@jilu1 Please kick off a build with your branch to check for artifact size increase + filepath scrubbing to make sure. Otherwise this is good to go. |
|
@KevinBKozan I have run builds after my change https://m2build-ur.devops.magento.com/job/All-User-Requested-Tests/24447/ and font looks much better than before. The link is in MQE ticket. |
|
Very nice, didn't see it. Approved then! |
Description
Fixed Issues (if relevant)
Contribution checklist