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

Tighten up CLI parsing #171

Closed
IngwiePhoenix opened this issue Dec 20, 2015 · 11 comments
Closed

Tighten up CLI parsing #171

IngwiePhoenix opened this issue Dec 20, 2015 · 11 comments
Labels
Milestone

Comments

@IngwiePhoenix
Copy link

I have a multi-language project, so I want to organize my tests inside a main test folder.

test/
  - php/
    - peridot.php
    - spec/
  - nodejs/
    - mocha.js
    - spec/

I know that I can use --configuration=test/php/peridot.php to tell it where to look for the config. But how do I configure the folder containing tests?

@brianium
Copy link
Member

You can always point to the directory when running peridot:

$ peridot  --configuration=test/php/peridot.php test/php/spec

Or the configuration file can configure the default directory like this

Then you would just have to specify the path to the config file.

Does this help?

@IngwiePhoenix
Copy link
Author

IngwiePhoenix commented Dec 20, 2015 via email

@IngwiePhoenix
Copy link
Author

Ah yeah, one little side question:

        $config->setGrep('*.feature.php');

Can I define multiple greps? I want to test bigger parts using the example DSL, and general stuff using regular statements. So I kinda need to have both, *.feature.php and *.spec.php. Can I do that?

@IngwiePhoenix
Copy link
Author

Weird. I actually run into troubble.

<?php
$autoloadPath = "php_modules/autoload.php";
$rootPath = __DIR__;
while(!file_exists("$rootPath/$autoloadPath")) {
    $rootPath = realpath("$rootPath/..");
}
require_once "$rootPath/$autoloadPath";

use Peridot\Console\Environment;
use expect\peridot\ExpectPlugin;

return function($emitter) {
    $eventEmitter->on('peridot.start', function (Environment $environment) {
        $p = __DIR__."/tests";
        $environment
            ->getDefinition()
            ->getArgument('path')
            ->setDefault($p);
    });

    # Plugins
    ExpectPlugin::create()->registerTo($emitter);

    $emitter->on('peridot.configure', function($config) {
        $config->setDsl(__DIR__.'/Dsl/feature.dsl.php');
        $config->setGrep('*.test.php');
        $config->setReporter("feature");
    });

    $emitter->on('peridot.reporters', function($input, $reporters) {
        $reporters->register('feature', 'A feature reporter', 'BIRD3\Test\Php\Dsl\FeatureReporter');
    });
};

Peridot is executed through an NPM script, and output looks like so:

[email protected] ~/W/BIRD3 $ npm run test:php

> [email protected] test:php /Users/Ingwie/Work/BIRD3
> php_modules/bin/peridot --configuration=test/php/peridot.php

PHP Fatal error:  Class 'Silex\Application' not found in /Users/Ingwie/Work/BIRD3/php_modules/peridot-php/peridot-httpkernel-plugin/app/app.php on line 5
PHP Stack trace:
PHP   1. {main}() /Users/Ingwie/Work/BIRD3/php_modules/peridot-php/peridot/bin/peridot:0
PHP   2. Peridot\Console\Application->run() /Users/Ingwie/Work/BIRD3/php_modules/peridot-php/peridot/bin/peridot:45
PHP   3. Symfony\Component\Console\Application->run() /Users/Ingwie/Work/BIRD3/php_modules/peridot-php/peridot/src/Console/Application.php:63
PHP   4. Peridot\Console\Application->doRun() /Users/Ingwie/Work/BIRD3/php_modules/symfony/console/Application.php:120
PHP   5. Symfony\Component\Console\Application->doRun() /Users/Ingwie/Work/BIRD3/php_modules/peridot-php/peridot/src/Console/Application.php:84
PHP   6. Symfony\Component\Console\Application->doRunCommand() /Users/Ingwie/Work/BIRD3/php_modules/symfony/console/Application.php:189
PHP   7. Symfony\Component\Console\Command\Command->run() /Users/Ingwie/Work/BIRD3/php_modules/symfony/console/Application.php:838
PHP   8. Peridot\Console\Command->execute() /Users/Ingwie/Work/BIRD3/php_modules/symfony/console/Command/Command.php:256
PHP   9. Peridot\Console\Command->getResult() /Users/Ingwie/Work/BIRD3/php_modules/peridot-php/peridot/src/Console/Command.php:149
PHP  10. Peridot\Runner\SuiteLoader->load() /Users/Ingwie/Work/BIRD3/php_modules/peridot-php/peridot/src/Console/Command.php:174
PHP  11. include() /Users/Ingwie/Work/BIRD3/php_modules/peridot-php/peridot/src/Runner/SuiteLoader.php:41
PHP  12. describe() /Users/Ingwie/Work/BIRD3/php_modules/peridot-php/peridot-httpkernel-plugin/app/specs/no-plugin.spec.php:34
PHP  13. Peridot\Runner\Context->addSuite() /Users/Ingwie/Work/BIRD3/php_modules/peridot-php/peridot/src/Dsl.php:12
PHP  14. Peridot\Core\Suite->define() /Users/Ingwie/Work/BIRD3/php_modules/peridot-php/peridot/src/Runner/Context.php:96
PHP  15. call_user_func_array:{/Users/Ingwie/Work/BIRD3/php_modules/peridot-php/peridot/src/Core/Suite.php:64}() /Users/Ingwie/Work/BIRD3/php_modules/peridot-php/peridot/src/Core/Suite.php:64
PHP  16. Peridot\Core\Scope->{closure:/Users/Ingwie/Work/BIRD3/php_modules/peridot-php/peridot-httpkernel-plugin/app/specs/no-plugin.spec.php:9-34}() /Users/Ingwie/Work/BIRD3/php_modules/peridot-php/peridot/src/Core/Suite.php:64
PHP  17. include() /Users/Ingwie/Work/BIRD3/php_modules/peridot-php/peridot-httpkernel-plugin/app/specs/no-plugin.spec.php:12

Why is it trying to load a test suite that I didn't even specify...?

@IngwiePhoenix
Copy link
Author

Oh, looky look. Actually the --configuration option is broken.

[email protected] ~/W/BIRD3 $ peridot --configuration=../peridot-dsl-example/peridot.php --reporters

    spec - hierarchical spec list

[email protected] ~/W/BIRD3 $ cd ../peridot-dsl-example/
[email protected] ~/W/peridot-dsl-example $ peridot --reporters

    spec - hierarchical spec list
    feature - A feature reporter

@IngwiePhoenix
Copy link
Author

Nevermind. I found the problem and its rather stupid, but something you should be aware of.

When opening Peridot's help:

[email protected] ~/W/BIRD3 $ peridot --help
Usage:
  peridot [options] [files]

Options:
  -g, --grep=GREP                    Run tests matching <pattern> (default: *.spec.php)
  -C, --no-colors                    Disable output colors
      --force-colors                 Force output colors
  -r, --reporter=REPORTER            Select which reporter to use (default: spec)
  -b, --bail                         Stop on failure
  -c, --configuration=CONFIGURATION  A php file containing peridot configuration
      --reporters                    List all available reporters
  -V, --version                      Display the Peridot version number
  -h, --help                         Display this help message.

It tells me to use an equal (=) sign to specify my configuration file. But if specify without the sign, everything works as expected and my peridot.php is read and understood as it should.

I have no idea why this is a thing, but its important to let you know.

@brianium
Copy link
Member

Ah. Thanks for finding this! A PR to address the help screen would definitely be welcome. I did not realize an = sign broke things. It might also be an issue with how peridot parses the confit switch. Peridot uses the Symfony console library for most of the CLI, but it parses the configuration option separately to load the confit before the Symfony app is created

@IngwiePhoenix
Copy link
Author

I highly suggest using a library for parsing commandline switches.

That ensures that specific behaviour is granted. As far as I know, Symfony does even have such functions, without having to bootstrap a Console app first.

When I find some time I will see if I can fix the current method with a PR.

@IngwiePhoenix
Copy link
Author

Just looked at https://github.com/peridot-php/peridot/blob/master/src/Console/CliOptionParser.php

The way you are parsing arguments would've to be overwritten entirely in order to make = work.

Basically:

  • Iterate over the arguments you got, and check if they have an equal sign. If so, split them.
  • split each found argument into a new array.
  • Then use your old parsing method. Now that the equal signs are removed and replaced with a space, so to say, your old - current - algorythm should do the trick

@brianium brianium changed the title Folder structure Tighten up CLI parsing Jul 12, 2016
@brianium brianium added the bug label Jul 12, 2016
@brianium
Copy link
Member

Peridot does a small amount of manual parsing of CLI options to allow the plugin system to register additional command line switches and options. However, the manual parsing allows for some error as pointed out by @IngwiePhoenix. Following Ingwie's suggestions with regards to how we parse the --config switch would make this more solid :). I've switched the title of this issue and flagged it as a bug.

@ezzatron
Copy link
Contributor

Released in 1.19.0 🎉

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

No branches or pull requests

3 participants