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.

Sign up for GitHub

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

Jump to bottom

feat: Improve the requirement checker constraint messages #877

Merged
merged 4 commits into from
Feb 12, 2023
Merged

feat: Improve the requirement checker constraint messages #877

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
bd59394
WIP
theofidry Feb 12, 2023
fa2daf2
Merge remote-tracking branch 'upstream/main' into fix/req-checker-mes…
theofidry Feb 12, 2023
7886f58
fix
theofidry Feb 12, 2023
07af6b4
fix
theofidry Feb 12, 2023
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
18 changes: 9 additions & 9 deletions requirement-checker/fixtures/fail-complete/expected-boxmin-stderr.dist
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,14 +8,10 @@ Box Requirements Checker

> Checking Box requirements:
✘ The application requires a version matching "^10".
✔ The application requires the extension "phar". Enable it or install a
polyfill.
✔ The package "package-with-extensions" requires the extension "json". Enable
it or install a polyfill.
✘ The package "package-with-extensions" requires the extension "ldap". Enable
it or install a polyfill.
✔ The application requires the extension "phar".
✔ The package "package-with-extensions" requires the extension "json".
✘ The package "package-with-extensions" requires the extension "ldap".
✘ The package "package-with-extensions" requires the extension "random".
Enable it or install a polyfill.


[ERROR] Your system is not ready to run the application.
Expand All @@ -25,6 +21,10 @@ Fix the following mandatory requirements:
=========================================

* The application requires a version matching "^10".
* The package "package-with-extensions" requires the extension "ldap".
* The package "package-with-extensions" requires the extension "random".
* The package "package-with-extensions" requires the extension "ldap". You
either need to enable it or request the application to be shipped with a
polyfill for this extension.
* The package "package-with-extensions" requires the extension "random". You
either need to enable it or request the application to be shipped with a
polyfill for this extension.

18 changes: 9 additions & 9 deletions requirement-checker/fixtures/fail-complete/expected-composermin-stderr.dist
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,14 +8,10 @@ Box Requirements Checker

> Checking Box requirements:
✘ The application requires a version matching "^10".
✔ The application requires the extension "phar". Enable it or install a
polyfill.
✔ The package "package-with-extensions" requires the extension "json". Enable
it or install a polyfill.
✘ The package "package-with-extensions" requires the extension "ldap". Enable
it or install a polyfill.
✔ The application requires the extension "phar".
✔ The package "package-with-extensions" requires the extension "json".
✘ The package "package-with-extensions" requires the extension "ldap".
✘ The package "package-with-extensions" requires the extension "random".
Enable it or install a polyfill.


[ERROR] Your system is not ready to run the application.
Expand All @@ -25,6 +21,10 @@ Fix the following mandatory requirements:
=========================================

* The application requires a version matching "^10".
* The package "package-with-extensions" requires the extension "ldap".
* The package "package-with-extensions" requires the extension "random".
* The package "package-with-extensions" requires the extension "ldap". You
either need to enable it or request the application to be shipped with a
polyfill for this extension.
* The package "package-with-extensions" requires the extension "random". You
either need to enable it or request the application to be shipped with a
polyfill for this extension.

6 changes: 3 additions & 3 deletions requirement-checker/fixtures/fail-conflict/expected-boxmin-stderr.dist
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,7 @@ Box Requirements Checker
WARNING: No configuration file (php.ini) used by PHP!

> Checking Box requirements:
✔ The application requires the extension "phar". Enable it or install a
polyfill.
✔ The application requires the extension "phar".
✘ The application conflicts with the extension "pdo".


Expand All @@ -18,5 +17,6 @@ polyfill.
Fix the following mandatory requirements:
=========================================

* The application conflicts with the extension "pdo". Disable it.
* The application conflicts with the extension "pdo". You need to disable it in
order to run this application.

3 changes: 1 addition & 2 deletions requirement-checker/fixtures/pass-complete/expected-boxmin-stderr.dist
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,7 @@ Box Requirements Checker

> Checking Box requirements:
✔ The application requires a version matching ">=5.3".
✔ The application requires the extension "phar". Enable it or install a
polyfill.
✔ The application requires the extension "phar".


[OK] Your system is ready to run the application.
Expand Down
3 changes: 1 addition & 2 deletions requirement-checker/fixtures/pass-complete/expected-composermin-stderr.dist
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,7 @@ Box Requirements Checker

> Checking Box requirements:
✔ The application requires a version matching ">=5.3".
✔ The application requires the extension "phar". Enable it or install a
polyfill.
✔ The application requires the extension "phar".


[OK] Your system is ready to run the application.
Expand Down
14 changes: 6 additions & 8 deletions src/RequirementChecker/Requirement.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,27 +57,26 @@ public static function forPHP(string $requiredPhpVersion, ?string $packageName):

public static function forRequiredExtension(string $extension, ?string $packageName): self
{
// TODO: review the message & help message
return new self(
'extension',
$extension,
null === $packageName
? sprintf(
'The application requires the extension "%s". Enable it or install a polyfill.',
'The application requires the extension "%s".',
$extension,
)
: sprintf(
'The package "%s" requires the extension "%s". Enable it or install a polyfill.',
'The package "%s" requires the extension "%s".',
$packageName,
$extension,
),
null === $packageName
? sprintf(
'The application requires the extension "%s".',
'The application requires the extension "%s". You either need to enable it or request the application to be shipped with a polyfill for this extension.',
$extension,
)
: sprintf(
'The package "%s" requires the extension "%s".',
'The package "%s" requires the extension "%s". You either need to enable it or request the application to be shipped with a polyfill for this extension.',
$packageName,
$extension,
),
Expand All @@ -86,7 +85,6 @@ public static function forRequiredExtension(string $extension, ?string $packageN

public static function forConflictingExtension(string $extension, ?string $packageName): self
{
// TODO: review the message & help message
return new self(
'extension-conflict',
$extension,
Expand All @@ -102,11 +100,11 @@ public static function forConflictingExtension(string $extension, ?string $packa
),
null === $packageName
? sprintf(
'The application conflicts with the extension "%s". Disable it.',
'The application conflicts with the extension "%s". You need to disable it in order to run this application.',
$extension,
)
: sprintf(
'The package "%s" conflicts with the extension "%s". Disable it.',
'The package "%s" conflicts with the extension "%s". You need to disable it in order to run this application.',
$packageName,
$extension,
),
Expand Down
12 changes: 6 additions & 6 deletions tests/RequirementChecker/RequirementTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,8 +58,8 @@ public function test_it_can_be_created_for_an_extension_constraint(): void
$expected = [
'type' => 'extension',
'condition' => 'mbstring',
'message' => 'The application requires the extension "mbstring". Enable it or install a polyfill.',
'helpMessage' => 'The application requires the extension "mbstring".',
'message' => 'The application requires the extension "mbstring".',
'helpMessage' => 'The application requires the extension "mbstring". You either need to enable it or request the application to be shipped with a polyfill for this extension.',
];

self::assertSame($expected, $requirement->toArray());
Expand All @@ -72,8 +72,8 @@ public function test_it_can_be_created_for_an_extension_constraint_for_a_package
$expected = [
'type' => 'extension',
'condition' => 'mbstring',
'message' => 'The package "box/test" requires the extension "mbstring". Enable it or install a polyfill.',
'helpMessage' => 'The package "box/test" requires the extension "mbstring".',
'message' => 'The package "box/test" requires the extension "mbstring".',
'helpMessage' => 'The package "box/test" requires the extension "mbstring". You either need to enable it or request the application to be shipped with a polyfill for this extension.',
];

self::assertSame($expected, $requirement->toArray());
Expand All @@ -87,7 +87,7 @@ public function test_it_can_be_created_for_a_conflicting_extension_constraint():
'type' => 'extension-conflict',
'condition' => 'mbstring',
'message' => 'The application conflicts with the extension "mbstring".',
'helpMessage' => 'The application conflicts with the extension "mbstring". Disable it.',
'helpMessage' => 'The application conflicts with the extension "mbstring". You need to disable it in order to run this application.',
];

self::assertSame($expected, $requirement->toArray());
Expand All @@ -101,7 +101,7 @@ public function test_it_can_be_created_for_a_conflicting_extension_constraint_fo
'type' => 'extension-conflict',
'condition' => 'mbstring',
'message' => 'The package "box/test" conflicts with the extension "mbstring".',
'helpMessage' => 'The package "box/test" conflicts with the extension "mbstring". Disable it.',
'helpMessage' => 'The package "box/test" conflicts with the extension "mbstring". You need to disable it in order to run this application.',
];

self::assertSame($expected, $requirement->toArray());
Expand Down
8 changes: 4 additions & 4 deletions tests/RequirementChecker/RequirementsDumperTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -141,15 +141,15 @@ public static function jsonAndLockContentsProvider(): iterable
array (
'type' => 'extension',
'condition' => 'zlib',
'message' => 'The application requires the extension "zlib". Enable it or install a polyfill.',
'helpMessage' => 'The application requires the extension "zlib".',
'message' => 'The application requires the extension "zlib".',
'helpMessage' => 'The application requires the extension "zlib". You either need to enable it or request the application to be shipped with a polyfill for this extension.',
),
2 =>
array (
'type' => 'extension',
'condition' => 'json',
'message' => 'The package "acme/foo" requires the extension "json". Enable it or install a polyfill.',
'helpMessage' => 'The package "acme/foo" requires the extension "json".',
'message' => 'The package "acme/foo" requires the extension "json".',
'helpMessage' => 'The package "acme/foo" requires the extension "json". You either need to enable it or request the application to be shipped with a polyfill for this extension.',
),
);
PHP,
Expand Down