Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: spark make:test creates test files in /tests/ directory v2 #8388

Merged
merged 7 commits into from
Jan 23, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 4 additions & 3 deletions system/Autoloader/Autoloader.php
Original file line number Diff line number Diff line change
Expand Up @@ -63,14 +63,14 @@ class Autoloader
/**
* Stores namespaces as key, and path as values.
*
* @var array<string, array<string>>
* @var array<string, list<string>>
*/
protected $prefixes = [];

/**
* Stores class name as key, and path as values.
*
* @var array<string, string>
* @var array<class-string, string>
*/
protected $classmap = [];

Expand Down Expand Up @@ -215,7 +215,8 @@ public function addNamespace($namespace, ?string $path = null)
*
* If a prefix param is set, returns only paths to the given prefix.
*
* @return array
* @return array<string, list<string>>|list<string>
* @phpstan-return ($prefix is null ? array<string, list<string>> : list<string>)
*/
public function getNamespace(?string $prefix = null)
{
Expand Down
10 changes: 5 additions & 5 deletions system/Autoloader/FileLocator.php
Original file line number Diff line number Diff line change
Expand Up @@ -175,6 +175,8 @@ public function getClassname(string $file): string
* 'app/Modules/foo/Config/Routes.php',
* 'app/Modules/bar/Config/Routes.php',
* ]
*
* @return list<string>
*/
public function search(string $path, string $ext = 'php', bool $prioritizeApp = true): array
{
Expand Down Expand Up @@ -203,7 +205,7 @@ public function search(string $path, string $ext = 'php', bool $prioritizeApp =
}

// Remove any duplicates
return array_unique($foundPaths);
return array_values(array_unique($foundPaths));
}

/**
Expand Down Expand Up @@ -237,7 +239,7 @@ protected function getNamespaces()
foreach ($this->autoloader->getNamespace() as $prefix => $paths) {
foreach ($paths as $path) {
if ($prefix === 'CodeIgniter') {
$system = [
$system[] = [
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems like a functionally-impactful change. Was this a bug?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, I see: this is to enable both directories to map to \CodeIgniter\. Could this cause other problems for existing code?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No. The return value is originally an array list [['prefix' => prefix, 'path' => path], ...].
Adding an array for \CodeIgniter\ for test as a last element does no harm.

'prefix' => $prefix,
'path' => rtrim($path, '\\/') . DIRECTORY_SEPARATOR,
];
Expand All @@ -252,9 +254,7 @@ protected function getNamespaces()
}
}

$namespaces[] = $system;

return $namespaces;
return array_merge($namespaces, $system);
}

/**
Expand Down
32 changes: 20 additions & 12 deletions system/CLI/GeneratorTrait.php
Original file line number Diff line number Diff line change
Expand Up @@ -263,6 +263,25 @@ protected function basename(string $filename): string
* Parses the class name and checks if it is already qualified.
*/
protected function qualifyClassName(): string
{
$class = $this->normalizeInputClassName();

// Gets the namespace from input. Don't forget the ending backslash!
$namespace = $this->getNamespace() . '\\';

if (strncmp($class, $namespace, strlen($namespace)) === 0) {
return $class; // @codeCoverageIgnore
}

$directoryString = ($this->directory !== null) ? $this->directory . '\\' : '';

return $namespace . $directoryString . str_replace('/', '\\', $class);
}

/**
* Normalize input classname.
*/
private function normalizeInputClassName(): string
{
// Gets the class name from input.
$class = $this->params[0] ?? CLI::getSegment(2);
Expand Down Expand Up @@ -298,7 +317,7 @@ protected function qualifyClassName(): string
}

// Trims input, normalize separators, and ensure that all paths are in Pascalcase.
$class = ltrim(
return ltrim(
implode(
'\\',
array_map(
Expand All @@ -308,17 +327,6 @@ protected function qualifyClassName(): string
),
'\\/'
);

// Gets the namespace from input. Don't forget the ending backslash!
$namespace = $this->getNamespace() . '\\';

if (strncmp($class, $namespace, strlen($namespace)) === 0) {
return $class; // @codeCoverageIgnore
}

$directoryString = ($this->directory !== null) ? $this->directory . '\\' : '';

return $namespace . $directoryString . str_replace('/', '\\', $class);
}

/**
Expand Down
108 changes: 106 additions & 2 deletions system/Commands/Generators/TestGenerator.php
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,9 @@
namespace CodeIgniter\Commands\Generators;

use CodeIgniter\CLI\BaseCommand;
use CodeIgniter\CLI\CLI;
use CodeIgniter\CLI\GeneratorTrait;
use Config\Services;

/**
* Generates a skeleton command file.
Expand Down Expand Up @@ -66,7 +68,8 @@ class TestGenerator extends BaseCommand
* @var array<string, string>
*/
protected $options = [
'--force' => 'Force overwrite existing file.',
'--namespace' => 'Set root namespace. Default: "Tests".',
'--force' => 'Force overwrite existing file.',
];

/**
Expand All @@ -76,9 +79,110 @@ public function run(array $params)
{
$this->component = 'Test';
$this->template = 'test.tpl.php';
$this->namespace = 'Tests';

$this->classNameLang = 'CLI.generator.className.test';
$this->generateClass($params);
}

/**
* Gets the namespace from input or the default namespace.
*/
protected function getNamespace(): string
{
if ($this->namespace !== null) {
return $this->namespace;
}

if ($this->getOption('namespace') !== null) {
return trim(
str_replace(
'/',
'\\',
$this->getOption('namespace')
),
'\\'
);
}

$class = $this->normalizeInputClassName();
$classPaths = explode('\\', $class);

$namespaces = Services::autoloader()->getNamespace();

while ($classPaths !== []) {
array_pop($classPaths);
$namespace = implode('\\', $classPaths);

foreach (array_keys($namespaces) as $prefix) {
if ($prefix === $namespace) {
// The input classname is FQCN, and use the namespace.
return $namespace;
}
}
}

return 'Tests';
}

/**
* Builds the test file path from the class name.
*
* @param string $class namespaced classname.
*/
protected function buildPath(string $class): string
{
$namespace = $this->getNamespace();

$base = $this->searchTestFilePath($namespace);

if ($base === null) {
CLI::error(
lang('CLI.namespaceNotDefined', [$namespace]),
'light_gray',
'red'
);
CLI::newLine();

return '';
}

$realpath = realpath($base);
$base = ($realpath !== false) ? $realpath : $base;

$file = $base . DIRECTORY_SEPARATOR
. str_replace(
'\\',
DIRECTORY_SEPARATOR,
trim(str_replace($namespace . '\\', '', $class), '\\')
) . '.php';

return implode(
DIRECTORY_SEPARATOR,
array_slice(
explode(DIRECTORY_SEPARATOR, $file),
0,
-1
)
) . DIRECTORY_SEPARATOR . $this->basename($file);
}

/**
* Returns test file path for the namespace.
*/
private function searchTestFilePath(string $namespace): ?string
{
$bases = Services::autoloader()->getNamespace($namespace);

$base = null;

foreach ($bases as $candidate) {
if (str_contains($candidate, '/tests/')) {
$base = $candidate;

break;
}
}

return $base;
}
}
12 changes: 6 additions & 6 deletions system/Commands/Utilities/Namespaces.php
Original file line number Diff line number Diff line change
Expand Up @@ -139,13 +139,13 @@ private function outputCINamespaces(array $params): array
$tbody = [];

foreach ($config->psr4 as $ns => $paths) {
if (array_key_exists('r', $params)) {
$pathOutput = $this->truncate($paths, $maxLength);
} else {
$pathOutput = $this->truncate(clean_path($paths), $maxLength);
}

foreach ((array) $paths as $path) {
if (array_key_exists('r', $params)) {
$pathOutput = $this->truncate($path, $maxLength);
} else {
$pathOutput = $this->truncate(clean_path($path), $maxLength);
}

$path = realpath($path) ?: $path;

$tbody[] = [
Expand Down
4 changes: 2 additions & 2 deletions system/Config/AutoloadConfig.php
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ class AutoloadConfig
* @var array<string, string>
*/
protected $corePsr4 = [
'CodeIgniter' => SYSTEMPATH,
'CodeIgniter' => [SYSTEMPATH, TESTPATH . 'system'],
'Config' => APPPATH . 'Config',
'Tests' => ROOTPATH . 'tests',
];
Expand All @@ -106,7 +106,7 @@ class AutoloadConfig
* searched for within one or more directories as they would if they
* were being autoloaded through a namespace.
*
* @var array<string, string>
* @var array<class-string, string>
*/
protected $coreClassmap = [
AbstractLogger::class => SYSTEMPATH . 'ThirdParty/PSR/Log/AbstractLogger.php',
Expand Down
1 change: 1 addition & 0 deletions tests/system/Commands/Utilities/NamespacesTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ public function testNamespacesCommandCodeIgniterOnly(): void
| Namespace | Path | Found? |
+---------------+-------------------------+--------+
| CodeIgniter | ROOTPATH/system | Yes |
| CodeIgniter | ROOTPATH/tests/system | Yes |
| Config | APPPATH/Config | Yes |
| Tests | ROOTPATH/tests | Yes |
| App | ROOTPATH/app | Yes |
Expand Down
1 change: 1 addition & 0 deletions user_guide_src/source/cli/cli_generators.rst
Original file line number Diff line number Diff line change
Expand Up @@ -248,6 +248,7 @@ Argument:

Options:
========
* ``--namespace``: Set the root namespace. Defaults to value of ``Tests``.
* ``--force``: Set this flag to overwrite existing files on destination.

make:migration
Expand Down
Loading