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: Add a info:requirements command #1273

Open
wants to merge 19 commits into
base: main
Choose a base branch
from

Conversation

theofidry
Copy link
Member

@theofidry theofidry commented Dec 15, 2023

Related to #1237.

  • Find a style that is pleasing
  • Extract all noises into separate PRs
  • Add unit tests (not an integration test)

@theofidry theofidry marked this pull request as draft December 15, 2023 08:38
theofidry added a commit that referenced this pull request Mar 11, 2024
This is a preparatory step for #1273. The goal is to be able to provide _all_ the requirements. The idea being that one may be interested in seeing this full list for debugging purpose, rather than the strict final one where the provided extensions removed the required extensions from the calculated requirements.
@theofidry theofidry marked this pull request as ready for review March 12, 2024 20:25
@theofidry
Copy link
Member Author

@llaville any opinion regarding the output?

Screenshot 2024-03-12 at 21 38 04

(you can also check the text output in the tests)

@llaville
Copy link
Contributor

Hello @theofidry
I've just tried to test your fork (feat/req-info branch), but I got some issues with it.

After :

  1. cloning it with command git clone -b feat/req-info https://github.com/theofidry/box box-fidry
  2. install it via composer because there is a composer.lock available : composer install

I run test on commit 6bf9e36

devilbox@php-8.2.16 in /shared/backups/github/box-fidry $ git log -n 1
commit 6bf9e36eb0dedf48daecdd54cb51065ea48ccabc (HEAD -> feat/req-info, origin/feat/req-info)
Author: Théo FIDRY <theo.fidry@gmail.com>
Date:   Tue Mar 12 21:25:00 2024 +0100

    fix tests

First use case : run info-req command on your clone, and got

 // Loading the configuration file "/shared/backups/github/box-fidry/box.json.dist".


Fatal error: Uncaught TypeError: strcmp(): Argument #1 ($string1) must be of type string, null given in /shared/backups/github/box-fidry/src/RequirementChecker/RequirementsBuilder.php on line 140

TypeError: strcmp(): Argument #1 ($string1) must be of type string, null given in /shared/backups/github/box-fidry/src/RequirementChecker/RequirementsBuilder.php on line 140

Call Stack:
    0.0002    1969952   1. {main}() /shared/backups/github/box-fidry/bin/box:0
    0.0720   10349352   2. Fidry\Console\Application\ApplicationRunner->run() /shared/backups/github/box-fidry/bin/box:60
    0.0720   10349352   3. Fidry\Console\Bridge\Application\SymfonyApplication->run() /shared/backups/github/box-fidry/vendor/fidry/console/src/Application/ApplicationRunner.php:83
    0.0723   10367360   4. Fidry\Console\Bridge\Application\SymfonyApplication->doRun() /shared/backups/github/box-fidry/vendor/symfony/console/Application.php:169
    0.0764   10635704   5. Fidry\Console\Bridge\Application\SymfonyApplication->doRunCommand() /shared/backups/github/box-fidry/vendor/symfony/console/Application.php:318
    0.0764   10635704   6. Fidry\Console\Bridge\Command\SymfonyCommand->run() /shared/backups/github/box-fidry/vendor/symfony/console/Application.php:1031
    0.0766   10644360   7. Fidry\Console\Bridge\Command\SymfonyCommand->execute() /shared/backups/github/box-fidry/vendor/symfony/console/Command/Command.php:279
    0.0766   10644360   8. KevinGH\Box\Console\Command\Info\RequirementsCommand->execute() /shared/backups/github/box-fidry/vendor/fidry/console/src/Bridge/Command/SymfonyCommand.php:103
    0.3300   44477384   9. KevinGH\Box\Console\Command\Info\RequirementsCommand->getAllRequirements() /shared/backups/github/box-fidry/src/Console/Command/Info/RequirementsCommand.php:95
    0.3300   44477384  10. KevinGH\Box\RequirementChecker\AppRequirementsFactory->createUnfiltered() /shared/backups/github/box-fidry/src/Console/Command/Info/RequirementsCommand.php:132
    0.3322   44547112  11. KevinGH\Box\RequirementChecker\RequirementsBuilder->all() /shared/backups/github/box-fidry/src/RequirementChecker/AppRequirementsFactory.php:43
    0.3322   44547112  12. KevinGH\Box\RequirementChecker\RequirementsBuilder->getSortedRequiredAndProvidedExtensions() /shared/backups/github/box-fidry/src/RequirementChecker/RequirementsBuilder.php:80
    0.3322   44548192  13. array_map() /shared/backups/github/box-fidry/src/RequirementChecker/RequirementsBuilder.php:136
    0.3322   44549424  14. KevinGH\Box\RequirementChecker\RequirementsBuilder::KevinGH\Box\RequirementChecker\{closure:/shared/backups/github/box-fidry/src/RequirementChecker/RequirementsBuilder.php:137-147}() /shared/backups/github/box-fidry/src/RequirementChecker/RequirementsBuilder.php:136
    0.3322   44550000  15. usort() /shared/backups/github/box-fidry/src/RequirementChecker/RequirementsBuilder.php:138
    0.3322   44550448  16. KevinGH\Box\RequirementChecker\RequirementsBuilder::KevinGH\Box\RequirementChecker\{closure:/shared/backups/github/box-fidry/src/RequirementChecker/RequirementsBuilder.php:140-143}() /shared/backups/github/box-fidry/src/RequirementChecker/RequirementsBuilder.php:138
    0.3322   44550608  17. strcmp() /shared/backups/github/box-fidry/src/RequirementChecker/RequirementsBuilder.php:140

Then I tried unit tests with vendor/bin/phpunit and got

PHPUnit 10.5.12 by Sebastian Bergmann and contributors.

Runtime:       PHP 8.2.16
Configuration: /shared/backups/github/box-fidry/phpunit.xml.dist
Random Seed:   1710310263

...............SSSSSSSSSSSSSSSSSSSSSSS..................SSSSSSS  63 / 943 (  6%)
SSSSSSSSSSSSSSSSS..................................SSSS........ 126 / 943 ( 13%)
............................................................... 189 / 943 ( 20%)
............................................................... 252 / 943 ( 26%)
...........SSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSS 315 / 943 ( 33%)
SSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSS...... 378 / 943 ( 40%)
........................................................SSSSSSS 441 / 943 ( 46%)
SSSSSSSSSSSSSSSSSSSSSSSSSSSSSS................................. 504 / 943 ( 53%)
............................................................... 567 / 943 ( 60%)
............................................................... 630 / 943 ( 66%)
..........................SSSSSSSSSS........................... 693 / 943 ( 73%)
.F............................................................. 756 / 943 ( 80%)
............................................................... 819 / 943 ( 86%)
............................................................... 882 / 943 ( 93%)
......................................................S......   943 / 943 (100%)

Time: 00:28.638, Memory: 64.00 MB

There was 1 failure:

1) KevinGH\Box\Console\ApplicationTest::test_get_helper_menu
Failed asserting that two strings are identical.
--- Expected
+++ Actual
@@ @@
   extract                 🚚  Extracts a given PHAR into a directory
   help                    Display help for a command
   info                    🔍  Displays information about the PHAR extension or file
-  info:requirements       Lists the application requirements found
   list                    List commands
   namespace               Prints the first part of the command namespace
   process                 ⚡  Applies the registered compactors and replacement values on a file
@@ @@
   composer:vendor-dir     Shows the Composer vendor-dir configured
  info
   info:general            🔍  Displays information about the PHAR extension or file
+  info:requirements       Lists the application requirements found
   info:signature          Displays the hash of the signature
 '

/shared/backups/github/box-fidry/vendor/fidry/console/src/Test/OutputAssertions.php:42
/shared/backups/github/box-fidry/tests/Console/ApplicationTest.php:143

FAILURES!
Tests: 943, Assertions: 1861, Failures: 1, Skipped: 208.

@theofidry
Copy link
Member Author

@llaville That the unit tests were failing was expected (in the sense the CI was failing too), but the error you have should already be fixed (there is a string cast at the very failing statement that you have). I extracted it in #1335 for better visibility (I originally kind of expected this issue when extracted it in #1334 but I did not have a failing case at that time)

@llaville
Copy link
Contributor

@theofidry Thanks for the fix, it run better now !

info:requirements on PHPLint 9.1
devilbox@php-8.2.16 in /shared/backups/github/box-fidry $ bin/box info:req -d /shared/backups/github/phplint/
 // Loading the configuration file "/shared/backups/github/phplint/box.json.dist".

The following PHP constraints were found:
┌─────────────┬────────┐
│ Constraints │ Source │
├─────────────┼────────┤
│ ^8.1        │ root   │
└─────────────┴────────┘

The following extensions constraints were found:
┌──────────┬─────────────────┬──────────────────────────────────┐
│ Type     │ Extension       │ Source                           │
├──────────┼─────────────────┼──────────────────────────────────┤
│ provided │ ctype           │ symfony/polyfill-ctype           │
│ required │ dom             │ root                             │
│ provided │ intl-grapheme   │ symfony/polyfill-intl-grapheme   │
│ provided │ intl-normalizer │ symfony/polyfill-intl-normalizer │
│ required │ json            │ root                             │
│ required │ mbstring        │ root                             │
│ provided │ mbstring        │ symfony/polyfill-mbstring        │
│ required │ zlib            │ root                             │
└──────────┴─────────────────┴──────────────────────────────────┘

The required and provided extensions constraints (see above) are resolved to compute the final required extensions.
The application requires the following extension constraints:
┌───────────┬────────┐
│ Extension │ Source │
├───────────┼────────┤
│ dom       │ root   │
│ json      │ root   │
│ zlib      │ root   │
└───────────┴────────┘

Conflicting extensions:
┌───────────┬───────────────────────────┐
│ Extension │ Source                    │
├───────────┼───────────────────────────┤
│ psr       │ symfony/service-contracts │
└───────────┴───────────────────────────┘

Now on improvement, what I'd like to have :

For a better readability on extension report used (sort results by type column)

The following extensions constraints were found:
┌──────────┬─────────────────┬──────────────────────────────────┐
│ Type     │ Extension       │ Source                           │
├──────────┼─────────────────┼──────────────────────────────────┤
│ required │ dom             │ root                             │
│ required │ json            │ root                             │
│ required │ mbstring        │ root                             │
│ required │ zlib            │ root                             │
│ provided │ ctype           │ symfony/polyfill-ctype           │
│ provided │ intl-grapheme   │ symfony/polyfill-intl-grapheme   │
│ provided │ intl-normalizer │ symfony/polyfill-intl-normalizer │
│ provided │ mbstring        │ symfony/polyfill-mbstring        │
└──────────┴─────────────────┴──────────────────────────────────┘

And the best one will be to add, as I've already explained, the note about zlib usage.

The following extensions constraints were found:
┌──────────┬─────────────────┬──────────────────────────────────────────────────────────────┐
│ Type     │ Extension       │ Source                                                       │
├──────────┼─────────────────┼──────────────────────────────────────────────────────────────┤
│ required │ dom             │ root                                                         │
│ required │ json            │ root                                                         │
│ required │ mbstring        │ root                                                         │
│ required │ zlib            │ suggested by humbug/box because files compression is enabled │
│ provided │ ctype           │ symfony/polyfill-ctype                                       │
│ provided │ intl-grapheme   │ symfony/polyfill-intl-grapheme                               │
│ provided │ intl-normalizer │ symfony/polyfill-intl-normalizer                             │
│ provided │ mbstring        │ symfony/polyfill-mbstring                                    │
└──────────┴─────────────────┴──────────────────────────────────────────────────────────────┘

@theofidry
Copy link
Member Author

For a better readability on extension report used (sort results by type column)

is it really better to sort by source rather than extension? Because if you want to evaluate why there is an extension or not that it what you would need

@theofidry
Copy link
Member Author

And the best one will be to add, as I've already #1237 (comment), the note about zlib usage.

I'll rename it to phar-compression instead of root, but I wouldn't use the phrasing "suggested by humbug/box", as Box does nothing there, it's the nature of compressing an archive.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants