Skip to content

Commit 9da15a8

Browse files
jilu1jilu1
authored
Merge pull request #3 from magento-commerce/MQE-2114
MQE-2114: Add additional checks to the annotations static check
2 parents 065b813 + e0dc6a1 commit 9da15a8

File tree

4 files changed

+278
-9
lines changed

4 files changed

+278
-9
lines changed

‎dev/tests/unit/Magento/FunctionalTestFramework/StaticCheck/AnnotationsCheckTest.php

+194
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,194 @@
1+
<?php
2+
/**
3+
* Copyright © Magento, Inc. All rights reserved.
4+
* See COPYING.txt for license details.
5+
*/
6+
7+
namespace tests\unit\Magento\FunctionalTestFramework\StaticCheck;
8+
9+
use AspectMock\Test as AspectMock;
10+
use Magento\FunctionalTestingFramework\StaticCheck\AnnotationsCheck;
11+
use Magento\FunctionalTestingFramework\Test\Objects\TestObject;
12+
use tests\unit\Util\MagentoTestCase;
13+
use ReflectionClass;
14+
15+
class AnnotationsCheckTest extends MagentoTestCase
16+
{
17+
/** @var AnnotationsCheck */
18+
private $staticCheck;
19+
20+
/** @var ReflectionClass*/
21+
private $staticCheckClass;
22+
23+
public function setUp(): void
24+
{
25+
$this->staticCheck = new AnnotationsCheck();
26+
$this->staticCheckClass = new \ReflectionClass($this->staticCheck);
27+
}
28+
29+
public function tearDown(): void
30+
{
31+
AspectMock::clean();
32+
}
33+
34+
public function testValidateRequiredAnnotationsNoError()
35+
{
36+
$annotations = [
37+
'features' => [
38+
0 => 'feature1'
39+
],
40+
'stories' => [
41+
0 => 'story1'
42+
],
43+
'description' => [
44+
'main' => 'description1',
45+
'test_files' => 'file1',
46+
'deprecated' => [
47+
0 => 'deprecated1'
48+
]
49+
],
50+
'severity' => [
51+
0 => 'severity1'
52+
],
53+
'title' => [
54+
0 => '[NO TESTCASEID]: title1'
55+
],
56+
];
57+
$expected = [];
58+
59+
// mock test object
60+
$test = AspectMock::double(
61+
TestObject::class,
62+
['getAnnotations' => $annotations, 'getName' => 'AnnotationsCheckTest']
63+
)->make();
64+
65+
$validateRequiredAnnotations = $this->staticCheckClass->getMethod('validateRequiredAnnotations');
66+
$validateRequiredAnnotations->setAccessible(true);
67+
68+
$validateRequiredAnnotations->invoke($this->staticCheck, $test);
69+
$this->assertEquals($expected, $this->staticCheck->getErrors());
70+
}
71+
72+
public function testValidateRequiredAnnotationsMissing()
73+
{
74+
$testCaseId = 'MC-12345';
75+
76+
$annotations = [
77+
'features' => [
78+
0 => 'feature1'
79+
],
80+
'stories' => [
81+
0 => 'story1'
82+
],
83+
'description' => [
84+
'test_files' => 'file1',
85+
'deprecated' => [
86+
0 => 'deprecated1'
87+
]
88+
],
89+
'title' => [
90+
0 => $testCaseId . ': title1'
91+
],
92+
'testCaseId' => [
93+
0 => $testCaseId
94+
]
95+
];
96+
$expected = [
97+
0 => [
98+
0 => 'Test AnnotationsCheckTest is missing the required annotations: description, severity'
99+
]
100+
];
101+
102+
// mock test object
103+
$test = AspectMock::double(
104+
TestObject::class,
105+
['getAnnotations' => $annotations, 'getName' => 'AnnotationsCheckTest']
106+
)->make();
107+
108+
$validateRequiredAnnotations = $this->staticCheckClass->getMethod('validateRequiredAnnotations');
109+
$validateRequiredAnnotations->setAccessible(true);
110+
111+
$validateRequiredAnnotations->invoke($this->staticCheck, $test);
112+
$this->assertEquals($expected, $this->staticCheck->getErrors());
113+
}
114+
115+
public function testValidateRequiredAnnotationsMissingNoTestCaseId()
116+
{
117+
$annotations = [
118+
'features' => [
119+
0 => 'feature1'
120+
],
121+
'stories' => [
122+
0 => 'story1'
123+
],
124+
'description' => [
125+
'test_files' => 'file1',
126+
'deprecated' => [
127+
0 => 'deprecated1'
128+
]
129+
],
130+
'title' => [
131+
0 => "[NO TESTCASEID]: \t"
132+
],
133+
];
134+
$expected = [
135+
0 => [
136+
0 => 'Test AnnotationsCheckTest is missing the required annotations: title, description, severity'
137+
]
138+
];
139+
140+
// mock test object
141+
$test = AspectMock::double(
142+
TestObject::class,
143+
['getAnnotations' => $annotations, 'getName' => 'AnnotationsCheckTest']
144+
)->make();
145+
146+
$validateRequiredAnnotations = $this->staticCheckClass->getMethod('validateRequiredAnnotations');
147+
$validateRequiredAnnotations->setAccessible(true);
148+
149+
$validateRequiredAnnotations->invoke($this->staticCheck, $test);
150+
$this->assertEquals($expected, $this->staticCheck->getErrors());
151+
}
152+
153+
public function testValidateRequiredAnnotationsEmpty()
154+
{
155+
$annotations = [
156+
'features' => [
157+
0 => 'feature1'
158+
],
159+
'stories' => [
160+
0 => 'story1'
161+
],
162+
'description' => [
163+
'main' => 'description1',
164+
'test_files' => 'file1',
165+
'deprecated' => [
166+
0 => 'deprecated1'
167+
]
168+
],
169+
'severity' => [
170+
0 => 'severity1'
171+
],
172+
'title' => [
173+
0 => ''
174+
],
175+
];
176+
$expected = [
177+
0 => [
178+
0 => 'Test AnnotationsCheckTest is missing the required annotations: title'
179+
]
180+
];
181+
182+
// mock test object
183+
$test = AspectMock::double(
184+
TestObject::class,
185+
['getAnnotations' => $annotations, 'getName' => 'AnnotationsCheckTest']
186+
)->make();
187+
188+
$validateRequiredAnnotations = $this->staticCheckClass->getMethod('validateRequiredAnnotations');
189+
$validateRequiredAnnotations->setAccessible(true);
190+
191+
$validateRequiredAnnotations->invoke($this->staticCheck, $test);
192+
$this->assertEquals($expected, $this->staticCheck->getErrors());
193+
}
194+
}

‎dev/tests/unit/Magento/FunctionalTestFramework/Test/Util/AnnotationExtractorTest.php

+62
Original file line numberDiff line numberDiff line change
@@ -123,6 +123,68 @@ public function testMissingAnnotations()
123123
);
124124
}
125125

126+
/**
127+
* Annotation extractor should throw warning when required annotations are empty
128+
*
129+
* @throws \Exception
130+
*/
131+
public function testEmptyRequiredAnnotations()
132+
{
133+
// Test Data, missing title, description, and severity
134+
$testAnnotations = [
135+
"nodeName" => "annotations",
136+
"features" => [
137+
[
138+
"nodeName" => "features",
139+
"value" => ""
140+
]
141+
],
142+
"stories" => [
143+
[
144+
"nodeName" => "stories",
145+
"value" => "TestStories"
146+
]
147+
],
148+
"title" => [
149+
[
150+
"nodeName" => "title",
151+
"value" => " "
152+
]
153+
],
154+
"description" => [
155+
[
156+
"nodeName" => "description",
157+
"value" => "\t"
158+
]
159+
],
160+
"severity" => [
161+
[
162+
"nodeName" => "severity",
163+
"value" => ""
164+
]
165+
],
166+
"group" => [
167+
[
168+
"nodeName" => "group",
169+
"value" => "TestGroup"
170+
]
171+
],
172+
];
173+
// Perform Test
174+
$extractor = new AnnotationExtractor();
175+
$returnedAnnotations = $extractor->extractAnnotations($testAnnotations, "testFileName");
176+
177+
// Asserts
178+
TestLoggingUtil::getInstance()->validateMockLogStatement(
179+
'warning',
180+
'DEPRECATION: Test testFileName is missing required annotations.',
181+
[
182+
'testName' => 'testFileName',
183+
'missingAnnotations' => "title, description, severity"
184+
]
185+
);
186+
}
187+
126188
public function testTestCaseIdUniqueness()
127189
{
128190
// Test Data

‎src/Magento/FunctionalTestingFramework/StaticCheck/AnnotationsCheck.php

+14-4
Original file line numberDiff line numberDiff line change
@@ -117,29 +117,39 @@ public function getOutput()
117117
*
118118
* @param TestObject $test
119119
* @return void
120+
* @SuppressWarnings(PHPMD.CyclomaticComplexity)
121+
* @SuppressWarnings(PHPMD.NPathComplexity)
120122
*/
121123
private function validateRequiredAnnotations($test)
122124
{
123125
$annotations = $test->getAnnotations();
124126
$missing = [];
125127

126128
$stories = $annotations['stories'] ?? null;
127-
if ($stories === null) {
129+
if ($stories === null || !isset($stories[0]) || empty(trim($stories[0]))) {
128130
$missing[] = "stories";
129131
}
130132

133+
$testCaseId = "[NO TESTCASEID]";
134+
if (isset($annotations['testCaseId'][0])) {
135+
$testCaseId = trim($annotations['testCaseId'][0]);
136+
}
137+
131138
$title = $annotations['title'] ?? null;
132-
if ($title === null) {
139+
if ($title === null
140+
|| !isset($title[0])
141+
|| empty(trim($title[0]))
142+
|| empty(trim(substr(trim($title[0]), strlen($testCaseId . ': '))))) {
133143
$missing[] = "title";
134144
}
135145

136146
$description = $annotations['description']['main'] ?? null;
137-
if ($description === null) {
147+
if ($description === null || empty(trim($description))) {
138148
$missing[] = "description";
139149
}
140150

141151
$severity = $annotations['severity'] ?? null;
142-
if ($severity === null) {
152+
if ($severity === null || !isset($severity[0]) || empty(trim($severity[0]))) {
143153
$missing[] = "severity";
144154
}
145155

‎src/Magento/FunctionalTestingFramework/Test/Util/AnnotationExtractor.php

+8-5
Original file line numberDiff line numberDiff line change
@@ -79,7 +79,7 @@ public function extractAnnotations($testAnnotations, $filename, $validateAnnotat
7979
// Only transform severity annotation
8080
if ($annotationKey == "severity") {
8181
$annotationObjects[$annotationKey] = $this->transformAllureSeverityToMagento(
82-
$annotationData[0]['value']
82+
trim($annotationData[0][self::ANNOTATION_VALUE])
8383
);
8484
continue;
8585
}
@@ -92,16 +92,17 @@ public function extractAnnotations($testAnnotations, $filename, $validateAnnotat
9292
}
9393

9494
foreach ($annotationData as $annotationValue) {
95-
$annotationValues[] = $annotationValue[self::ANNOTATION_VALUE];
95+
$annotationValues[] = trim($annotationValue[self::ANNOTATION_VALUE]);
9696
}
9797

9898
$annotationObjects[$annotationKey] = $annotationValues;
9999
}
100100

101-
$this->addTestCaseIdToTitle($annotationObjects, $filename);
102101
if ($validateAnnotations) {
103102
$this->validateMissingAnnotations($annotationObjects, $filename);
104103
}
104+
105+
$this->addTestCaseIdToTitle($annotationObjects, $filename);
105106
$this->addStoryTitleToMap($annotationObjects, $filename);
106107

107108
return $annotationObjects;
@@ -156,7 +157,9 @@ private function validateMissingAnnotations($annotationObjects, $filename)
156157
$missingAnnotations = [];
157158

158159
foreach (self::REQUIRED_ANNOTATIONS as $REQUIRED_ANNOTATION) {
159-
if (!array_key_exists($REQUIRED_ANNOTATION, $annotationObjects)) {
160+
if (!array_key_exists($REQUIRED_ANNOTATION, $annotationObjects)
161+
|| !isset($annotationObjects[$REQUIRED_ANNOTATION][0])
162+
|| empty($annotationObjects[$REQUIRED_ANNOTATION][0])) {
160163
$missingAnnotations[] = $REQUIRED_ANNOTATION;
161164
}
162165
}
@@ -258,7 +261,7 @@ public function validateTestCaseIdTitleUniqueness()
258261
public function validateSkippedIssues($issues, $filename)
259262
{
260263
foreach ($issues as $issueId) {
261-
if (empty($issueId['value'])) {
264+
if (empty(trim($issueId['value']))) {
262265
$message = "issueId for skipped tests cannot be empty. Test: $filename";
263266
throw new XmlException($message);
264267
}

0 commit comments

Comments
 (0)