Skip to content

Commit 209bc3c

Browse files
authored
Simplify Test classes (#6845)
1 parent c840074 commit 209bc3c

File tree

13 files changed

+485
-646
lines changed

13 files changed

+485
-646
lines changed

src/Codeception/Test/Cept.php

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -7,8 +7,8 @@
77
use Codeception\Exception\TestParseException;
88
use Codeception\Lib\Console\Message;
99
use Codeception\Lib\Parser;
10-
use Exception;
1110
use ParseError;
11+
use RuntimeException;
1212

1313
use function basename;
1414
use function file_get_contents;
@@ -48,11 +48,10 @@ public function preload(): void
4848
public function test(): void
4949
{
5050
$scenario = $this->getScenario();
51-
$testFile = $this->getMetadata()->getFilename();
5251
try {
53-
require $testFile;
52+
require $this->getFileName();
5453
} catch (ParseError $error) {
55-
throw new TestParseException($testFile, $error->getMessage(), $error->getLine());
54+
throw new TestParseException($this->getFileName(), $error->getMessage(), $error->getLine());
5655
}
5756
}
5857

@@ -69,8 +68,8 @@ public function toString(): string
6968
public function getSourceCode(): string
7069
{
7170
$fileName = $this->getFileName();
72-
if (!$sourceCode = file_get_contents($fileName)) {
73-
throw new Exception("Could not get content of file {$fileName}, please check its permissions.");
71+
if (!is_readable($fileName) || ($sourceCode = file_get_contents($fileName)) === false) {
72+
throw new RuntimeException("Could not read file {$fileName}, please check its permissions.");
7473
}
7574
return $sourceCode;
7675
}

src/Codeception/Test/Cest.php

Lines changed: 32 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -27,11 +27,9 @@
2727
use function file;
2828
use function implode;
2929
use function is_callable;
30-
use function method_exists;
3130
use function preg_replace;
3231
use function sprintf;
3332
use function strtolower;
34-
use function trim;
3533

3634
/**
3735
* Executes tests delivered in Cest format.
@@ -67,8 +65,8 @@ public function __construct(object $testInstance, string $methodName, string $fi
6765
$metadata->setParamsFromAttributes($methodAnnotations->attributes());
6866
$this->setMetadata($metadata);
6967
$this->testInstance = $testInstance;
70-
$this->testClass = $testInstance::class;
71-
$this->testMethod = $methodName;
68+
$this->testClass = $testInstance::class;
69+
$this->testMethod = $methodName;
7270
$this->createScenario();
7371
$this->parser = new Parser($this->getScenario(), $this->getMetadata());
7472
}
@@ -81,24 +79,26 @@ public function __clone(): void
8179
public function preload(): void
8280
{
8381
$this->scenario->setFeature($this->getSpecFromMethod());
84-
$code = $this->getSourceCode();
85-
$this->parser->parseFeature($code);
86-
$this->getMetadata()->getService('di')->injectDependencies($this->testInstance);
87-
88-
// add example params to feature
89-
if ($this->getMetadata()->getCurrent('example')) {
90-
$step = new Comment('', $this->getMetadata()->getCurrent('example'));
91-
$this->getScenario()->setFeature($this->getScenario()->getFeature() . ' | ' . $step->getArgumentsAsString(100));
82+
$this->parser->parseFeature($this->getSourceCode());
83+
$this->getDiService()->injectDependencies($this->testInstance);
84+
85+
if ($example = $this->getMetadata()->getCurrent('example')) {
86+
$step = new Comment('', $example);
87+
$this->scenario->setFeature(
88+
$this->scenario->getFeature() . ' | ' . $step->getArgumentsAsString(100)
89+
);
9290
}
9391
}
9492

9593
public function getSourceCode(): string
9694
{
9795
$method = new ReflectionMethod($this->testInstance, $this->testMethod);
9896
$startLine = $method->getStartLine() - 1; // it's actually - 1, otherwise you wont get the function() block
99-
$endLine = $method->getEndLine();
100-
$source = file($method->getFileName());
101-
return implode("", array_slice($source, $startLine, $endLine - $startLine));
97+
$lines = file($method->getFileName());
98+
return implode(
99+
'',
100+
array_slice($lines, $startLine, $method->getEndLine() - $startLine)
101+
);
102102
}
103103

104104
public function getSpecFromMethod(): string
@@ -111,18 +111,14 @@ public function getSpecFromMethod(): string
111111

112112
public function test(): void
113113
{
114-
$actorClass = $this->getMetadata()->getCurrent('actor');
115-
116-
if ($actorClass === null) {
117-
throw new ConfigurationException(
114+
$actorClass = $this->getMetadata()->getCurrent('actor')
115+
?? throw new ConfigurationException(
118116
'actor setting is missing in suite configuration. Replace `class_name` with `actor` in config to fix this'
119117
);
120-
}
121118

122-
/** @var Di $di */
123-
$di = $this->getMetadata()->getService('di');
119+
$di = $this->getDiService();
124120
$di->set($this->getScenario());
125-
$I = $di->instantiate($actorClass);
121+
$I = $di->instantiate($actorClass);
126122

127123
try {
128124
$this->executeHook($I, 'before');
@@ -177,12 +173,12 @@ protected function executeContextMethod(string $context, $I): void
177173
);
178174
}
179175

180-
protected function invoke($methodName, array $context): void
176+
protected function invoke(string $methodName, array $context): void
181177
{
182178
foreach ($context as $class) {
183-
$this->getMetadata()->getService('di')->set($class);
179+
$this->getDiService()->set($class);
184180
}
185-
$this->getMetadata()->getService('di')->injectDependencies($this->testInstance, $methodName, $context);
181+
$this->getDiService()->injectDependencies($this->testInstance, $methodName, $context);
186182
}
187183

188184
protected function executeTestMethod($I): void
@@ -191,14 +187,11 @@ protected function executeTestMethod($I): void
191187
throw new Exception("Method {$this->testMethod} can't be found in tested class");
192188
}
193189

194-
if ($this->getMetadata()->getCurrent('example')) {
195-
$this->invoke(
196-
$this->testMethod,
197-
[$I, $this->scenario, new Example($this->getMetadata()->getCurrent('example'))]
198-
);
199-
return;
190+
if ($example = $this->getMetadata()->getCurrent('example')) {
191+
$this->invoke($this->testMethod, [$I, $this->scenario, new Example($example)]);
192+
} else {
193+
$this->invoke($this->testMethod, [$I, $this->scenario]);
200194
}
201-
$this->invoke($this->testMethod, [$I, $this->scenario]);
202195
}
203196

204197
public function toString(): string
@@ -212,7 +205,7 @@ public function toString(): string
212205

213206
public function getSignature(): string
214207
{
215-
return $this->testClass . ":" . $this->testMethod;
208+
return "{$this->testClass}:{$this->testMethod}";
216209
}
217210

218211
public function getTestInstance(): object
@@ -279,4 +272,9 @@ public function getLinesToBeUsed(): array
279272

280273
return (new CodeCoverage())->linesToBeUsed($this->testClass, $this->testMethod);
281274
}
275+
276+
private function getDiService(): Di
277+
{
278+
return $this->getMetadata()->getService('di');
279+
}
282280
}

src/Codeception/Test/DataProvider.php

Lines changed: 55 additions & 76 deletions
Original file line numberDiff line numberDiff line change
@@ -19,83 +19,69 @@ class DataProvider
1919
{
2020
public static function getDataForMethod(ReflectionMethod $method, ?ReflectionClass $class = null, ?Actor $I = null): ?iterable
2121
{
22-
$testClass = self::getTestClass($method, $class);
22+
$testClass = self::getTestClass($method, $class);
2323
$testClassName = $testClass->getName();
24-
$methodName = $method->getName();
25-
26-
// example annotation
27-
$rawExamples = array_values(
28-
Annotation::forMethod($testClassName, $methodName)->fetchAll('example'),
29-
);
24+
$methodName = $method->getName();
25+
$annotation = Annotation::forMethod($testClassName, $methodName);
3026

27+
$data = [];
28+
$rawExamples = $annotation->fetchAll('example');
3129
if ($rawExamples !== []) {
32-
$rawExample = reset($rawExamples);
33-
if (is_string($rawExample)) {
34-
$result = array_map(
35-
static fn ($v): ?array => Annotation::arrayValue($v),
36-
$rawExamples
37-
);
38-
} else {
39-
$result = $rawExamples;
30+
$convert = is_string(reset($rawExamples));
31+
foreach ($rawExamples as $example) {
32+
$data[] = $convert ? Annotation::arrayValue($example) : $example;
4033
}
41-
} else {
42-
$result = [];
4334
}
4435

45-
// dataProvider annotation
46-
$dataProviderAnnotations = Annotation::forMethod($testClassName, $methodName)->fetchAll('dataProvider');
47-
// lowercase for back compatible
48-
if ($dataProviderAnnotations === []) {
49-
$dataProviderAnnotations = Annotation::forMethod($testClassName, $methodName)->fetchAll('dataprovider');
50-
}
36+
$providers = array_merge(
37+
$annotation->fetchAll('dataProvider'),
38+
$annotation->fetchAll('dataprovider')
39+
);
5140

52-
if ($result === [] && $dataProviderAnnotations === []) {
41+
if ($data === [] && $providers === []) {
5342
return null;
5443
}
5544

56-
foreach ($dataProviderAnnotations as $dataProviderAnnotation) {
57-
[$dataProviderClassName, $dataProviderMethodName] = self::parseDataProviderAnnotation(
58-
$dataProviderAnnotation,
45+
foreach ($providers as $provider) {
46+
[$providerClass, $providerMethod] = self::parseDataProviderAnnotation(
47+
$provider,
5948
$testClassName,
60-
$methodName,
49+
$methodName
6150
);
6251

6352
try {
64-
$dataProviderMethod = new ReflectionMethod($dataProviderClassName, $dataProviderMethodName);
65-
if ($dataProviderMethod->isStatic()) {
66-
$dataProviderResult = call_user_func([$dataProviderClassName, $dataProviderMethodName], $I);
53+
$refMethod = new ReflectionMethod($providerClass, $providerMethod);
54+
55+
if ($refMethod->isStatic()) {
56+
$result = $providerClass::$providerMethod($I);
6757
} else {
68-
$testInstance = new $dataProviderClassName($dataProviderMethodName);
69-
70-
if ($dataProviderMethod->isPublic()) {
71-
$dataProviderResult = $testInstance->$dataProviderMethodName($I);
72-
} else {
73-
$dataProviderResult = ReflectionHelper::invokePrivateMethod(
74-
$testInstance,
75-
$dataProviderMethodName,
76-
[$I]
77-
);
78-
}
58+
$instance = new $providerClass($providerMethod);
59+
$result = $refMethod->isPublic()
60+
? $instance->$providerMethod($I)
61+
: ReflectionHelper::invokePrivateMethod($instance, $providerMethod, [$I]);
7962
}
8063

81-
foreach ($dataProviderResult as $key => $value) {
82-
if (is_int($key)) {
83-
$result [] = $value;
84-
} else {
85-
$result[$key] = $value;
86-
}
64+
if (!is_iterable($result)) {
65+
throw new InvalidTestException(
66+
"DataProvider '{$provider}' for {$testClassName}::{$methodName} " .
67+
'must return iterable data, got ' . gettype($result)
68+
);
8769
}
88-
} catch (ReflectionException) {
70+
71+
foreach ($result as $key => $value) {
72+
is_int($key) ? $data[] = $value : $data[$key] = $value;
73+
}
74+
} catch (ReflectionException $e) {
8975
throw new InvalidTestException(sprintf(
9076
"DataProvider '%s' for %s::%s is invalid or not callable",
91-
$dataProviderAnnotation,
77+
$provider,
9278
$testClassName,
9379
$methodName
94-
));
80+
), 0, $e);
9581
}
9682
}
9783

98-
return $result;
84+
return $data ?: null;
9985
}
10086

10187
/**
@@ -108,37 +94,30 @@ public static function parseDataProviderAnnotation(
10894
string $testMethodName,
10995
): array {
11096
$parts = explode('::', $annotation);
111-
if (count($parts) > 2) {
112-
throw new InvalidTestException(
113-
sprintf(
114-
'Data provider "%s" specified for %s::%s is invalid',
115-
$annotation,
116-
$testClassName,
117-
$testMethodName,
118-
)
119-
);
120-
}
12197

122-
if (count($parts) === 2) {
98+
if (count($parts) === 2 && $parts[0] !== '') {
12399
return $parts;
124100
}
101+
if (count($parts) === 1 || $parts[0] === '') {
102+
return [$testClassName, ltrim($parts[1] ?? $parts[0], ':')];
103+
}
125104

126-
return [
127-
$testClassName,
105+
throw new InvalidTestException(sprintf(
106+
'Data provider "%s" specified for %s::%s is invalid',
128107
$annotation,
129-
];
108+
$testClassName,
109+
$testMethodName
110+
));
130111
}
131112

132-
/**
133-
* Retrieves actual test class for dataProvider.
134-
*/
135-
private static function getTestClass(ReflectionMethod $dataProviderMethod, ?ReflectionClass $testClass): ReflectionClass
113+
private static function getTestClass(ReflectionMethod $method, ?ReflectionClass $class): ReflectionClass
136114
{
137-
$dataProviderDeclaringClass = $dataProviderMethod->getDeclaringClass();
138-
// data provider in abstract class?
139-
if ($dataProviderDeclaringClass->isAbstract() && $testClass instanceof ReflectionClass && $dataProviderDeclaringClass->name !== $testClass->name) {
140-
return $testClass;
141-
}
142-
return $dataProviderDeclaringClass;
115+
$declaringClass = $method->getDeclaringClass();
116+
117+
return $declaringClass->isAbstract()
118+
&& $class instanceof ReflectionClass
119+
&& $declaringClass->getName() !== $class->getName()
120+
? $class
121+
: $declaringClass;
143122
}
144123
}

0 commit comments

Comments
 (0)