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

Test.php suffix convention #33

Closed
adam-lynch opened this issue Mar 20, 2013 · 12 comments
Closed

Test.php suffix convention #33

adam-lynch opened this issue Mar 20, 2013 · 12 comments

Comments

@adam-lynch
Copy link
Contributor

All tests filenames must end with Test.php (as is enforced in the SuiteLoader).

I could be wrong, but should this always be done? Should it be done only if a .xml.dist file isn't given?

@dbaltas
Copy link
Contributor

dbaltas commented Mar 22, 2013

Filenames ending with Test.php is a strong PHPUnit convention.
Even running PHPUnit suite standalone would ignore php files not ending with this suffix when specifying a folder.

You could write a patch by setting $relaxTestPattern to true in the call of tryLoadTests
$this->tryLoadTests($path . DIRECTORY_SEPARATOR . $file, true);
https://github.com/brianium/paratest/blob/master/src/ParaTest/Runners/PHPUnit/SuiteLoader.php#L61

Alternatively, this option, relaxTestPattern could be configurable in the command line options, but I am not sure if Brian would like to go down this path.

Is there a good reason to not have your tests filenames ending with ...Test.php?

@brianium
Copy link
Contributor

I am of the same opinion as Dimitris. Unless there is overwhelming demand for this, I think we should leave it as is. The configuration via command line would be an acceptable solution if it were necessary. Let's close this for now and revisit it if we see some demand for it.

@adam-lynch
Copy link
Contributor Author

Ok, no problem. But just let me comment on a couple of things.

Is there a good reason to not have your tests filenames ending with ...Test.php?

It's not a massive problem for us since we're just beginning to write the (Selenium) test files we intend to parallelize, but it just breaks our convention of having our test directories mirror our source directories in structure and filenames (which we have for our set of existing plain PHPUnit tests).

Filenames ending with Test.php is a strong PHPUnit convention.
Even running PHPUnit suite standalone would ignore php files not ending with this suffix when specifying a folder.

I just noticed this because in our .xml.dist we have specified:

<testsuites>
    <testsuite name="Blah project">
        <directory suffix='.php'>blah/blah</directory>
        <directory suffix='.php'>blah2/blah</directory>
        <exclude>blah/blah/blah</exclude>
    </testsuite>
</testsuites>

Which is fine when ran regularly with PHPUnit but via Paratest, the suffix we have specified is ignored. So, I guess it's blocking a PHPUnit configuration feature. It's not a big problem for us but might be for someone with a large set of existing tests they want to parallelize.

@brianium
Copy link
Contributor

The configuration aspect of PHPUnit is more difficult to cover with Paratest. I think it would be awesome if we could support more of it. I started the work that supports the limited configuration here https://github.com/brianium/paratest/blob/master/src/ParaTest/Runners/PHPUnit/Configuration.php. Maybe this context is something we could begin to support?

@adam-lynch
Copy link
Contributor Author

Yeah I guessed that, since you'll basically be doing work PHPUnit will do anyway when it reads the dist itself.

Unless there's anyway to leverage the internal classes of PHPUnit itself?

@adam-lynch
Copy link
Contributor Author

This is really a frustrating issue. I got bitten by it just now again.

adam-lynch added a commit to adam-lynch/paratest that referenced this issue May 15, 2013
@brianium brianium reopened this May 15, 2013
@brianium
Copy link
Contributor

I am thinking on the best way to support this. The easy way is to allow suffix patterns to be specified via the cli. It might be worth looking into a better way to handle configuration concerns in general. It doesn't seem like it would be too difficult to make the suite loader aware of directories and their provided suffixes. The SuiteLoader already has access to the configuration that is parsing those suites. Maybe we can parse those suite nodes into more complete objects (instead of a dictionary of name => path pairs).

@brianium
Copy link
Contributor

@ghost ghost assigned brianium May 15, 2013
@giorgiosironi
Copy link
Contributor

I agree with @adam-lynch in expecting a phpunit.xml[.dist] suffix attributes to work. If one of the goals of the project is transparency, it shouldn't be necessary to add new syntax (command line options) for features already supported by PHPUnit.

@dbaltas
Copy link
Contributor

dbaltas commented May 20, 2013

True that.
Parsing PHPUnit configuration would cause less confusion.
I am sure other people like Adam would expect a configuration set there simply to work.

When we get to the point where paratest supports other testing frameworks,
we can add this option in paratest. It would default to PHPUnit config when set.
Similar to what we did with the bootstrap file #34.

@rusitschka
Copy link

I'd also highly appreciate the PHPUnit configuration approach. Here's an example from one of our test suites:

    <testsuites>
        <testsuite name="all">
            <directory suffix="Test.php">test/application</directory>
            <directory suffix="SharedTest.php">../mzlib/test/application</directory>
        </testsuite>
    </testsuites>

The paths are interpreted correctly but the suffixes are currently ignored by paratest.

julianseeger added a commit that referenced this issue Oct 17, 2014
Support suffix in test suite configurations, see #33
@julianseeger
Copy link
Contributor

Thanks to @rusitschka this is finally implemented.

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

No branches or pull requests

6 participants