This Bugzilla instance is a read-only archive of historic NetBeans bug reports. To report a bug in NetBeans please follow the project's instructions for reporting issues.

Bug 167519 - PHPUnit tests not executed if filename doesn't match class name
Summary: PHPUnit tests not executed if filename doesn't match class name
Status: RESOLVED FIXED
Alias: None
Product: php
Classification: Unclassified
Component: PHPUnit (show other bugs)
Version: 6.x
Hardware: PC Windows XP
: P3 blocker (vote)
Assignee: Tomas Mysik
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2009-06-23 10:11 UTC by marcusson
Modified: 2009-07-21 12:50 UTC (History)
0 users

See Also:
Issue Type: DEFECT
Exception Reporter:


Attachments
NetBeansSuite (1.37 KB, text/plain)
2009-07-17 09:12 UTC, Tomas Mysik
Details
API docs generated by PHP-Documentor (10.20 KB, application/x-compressed)
2009-07-17 19:51 UTC, marcusson
Details

Note You need to log in before you can comment on or make changes to this bug.
Description marcusson 2009-06-23 10:11:32 UTC
In NetBeans 6.7 RC3 running all PHPUnit tests using "Run" / "Test project" does not work if the filenames of the test
files don't match the class names.

If this issue is not fixed it will cause problems with existing PHPUnit test cases of projects that don't follow that
(undocumented) naming-convention.
This is especially important for Windows users, as in Windows filenames are not case-sensitive.

HowTo:
- write a class "Foo"
- write a test-case "FooTest" for "Foo" and save it to "footest.php"
- make sure the test-directory is set and "footest.php" appears in the list of "test files" in the "project" view
- select "Run" / "Test project"
- a message box will appear saying that no tests have been executed

Reason:
- in PHP the filename does not necessarily need to match the class name
- while users that know Java might think that to follow this naming-convention is a good idea, this is not a "you must"
but a "you may"

Description:
- if PHPUnit is given a class name, it tries to guess the name of the implementing file
- PHPUnit then expects the file name to match the class name (this comparison is case-sensitive)
- if the filename does not match "<ClassName>.php", PHPUnit reports that the test is not found

Solution:
- you need to give PHPUnit the filename NOT the class name!
- if so, PHPUnit will use the file and auto-detect the included test-cases, whatever they are

You may also want to look a the following code example for a generic PHPUnit test suite. It scans the test directory and
adds the test-cases by filename (!) not by class name.

class SuiteFull extends PHPUnit_Framework_TestSuite
{
    public static function suite()
    {
        $suite = new SuiteFull();
        foreach (glob('*.php') as $file)
        {
            $suite->addTestFile($file);
        }
        return $suite;
    }
}
Comment 1 Tomas Mysik 2009-07-15 13:54:23 UTC
Behaviour of PHPUnit, let me clarify. Two possible scenarios:

run exactly one unit test (SHIFT + F6)
- NB runs PHPUnit using "classname" "filename" (so in this scenario, it works)

run all the tests (ALT + F6)
- NB runs PHPUnit using "directory" (so in this scenario, it does not work because PHPUnit ignores that file)

So, sorry but no way to fix on NB side - please, consider submitting a report against PHPUnit.

However, keeping opened but lowering to P4 to have it as a reminder.

Thanks for reporting.
Comment 2 marcusson 2009-07-15 18:19:05 UTC
I understand, that running it on a directory does not do the job.
However, I don't think that the author will fix that within a reasonable time and we shouldn't wait for him to do so.

There is another solution for the case you stated here.

The easiest way to solve this issue is to let the user define a project specific test suite that should be run when he
clicks "Run" / "Test project".

If so, NetBeans should look for the specified test-suite and execute it. Only if none is specified, NetBeans may fall
back to the directory name.

Currently it's impossible to define a standard test-suite for a project in NetBeans.

Benefits:
- don't need to rely on PHPUnit to make it work
- will help to fix at least one other issue (#167521) along the way
- introduces support for PHPUnit Test-Suites with NetBeans, which also enhances NetBeans' support for PHPUnit as a whole

More Good reasons:
- For PHPUnit (as for JUnit) the definition of a test suite, that runs all tests, is quite common.

- Users may decide that running all tests takes too long. They may wish to run only a subset of all test-cases. Allowing
users to decide which test-suite to use, gives them a chance to get NetBeans to do this.

- This is also allowing users to circumvent a lot of issues that might come up in the future, as they always can
quick-fix their problem by writing their own test-suite.

- The PHP code for the generic test-suite class that I posted earlier, may also be used to auto-generate such a file, in
case you wish to include this feature in a future release. Other users might find that useful. The code is static, so
copy&paste will do.

- You will note that issue #167521 is also connected to this solution, as you won't be able to fix this issue without
writing your own test suite anyway. It can't be done by just tweaking the command-line call.
Comment 3 Tomas Mysik 2009-07-16 12:11:58 UTC
OK - I would propose this - similarly to bootstrap.php and configuration.xml, NB could generate all-tests.php (if not exists) in tests folder and then run it.
BTW your script must be improved to be recursive - do you want to contribute? ;)

Thanks.
Comment 4 Tomas Mysik 2009-07-16 12:13:50 UTC
Of course, it can easily happen that no tests will be run if one changes all-tests.php in a "bad" way... Maybe NB could fallback to the current behaviour in
such case.
Comment 5 marcusson 2009-07-17 01:17:15 UTC
Sure I want to contribute ;)
I will change the code so it scans directories recursively.

But I would suggest not to name the file "all-tests.php" and instead stick to the naming convention of file-name =
class-name. Also the suffix "test" is reserved for test-cases. I think the common suffix/prefix for test-suites is
"suite", so I would suggest "SuiteAll.php" or "AllSuite.php". (Actually I call my primary test-suite "SuiteFull.php",
but this is just a matter of taste.)

Here is the code. I added a full documentation including common PHP-Documentor tags, so you won't have to write this
yourself. You may still want to check it for typos though, as I'm not a native speaker. Change it as you see fit.
I also changed the code to scan for the common suffix *test.php. This is since PHPUnit will produce a warning for each
added file that does not contain test-cases. These warnings will be reported as test-failures. We should avoid that.

Besides, you will also note that I changed the code to use the full system path for directories and files. I won't use
'.' but dirname(__FILE__). This is because you shouldn't rely on the current working directory to be the directory where
the test-file resides.

I tested this generic test-suite with my own test-cases on a Windows system, but I believe it should work just as well
within a Linux environment.

<?php
/**
 * PHPUnit test-suite.
 *
 * @package  test
 */

/**
 * Generic test suite containing all tests.
 *
 * Recursively scans the test-directory and it's
 * sub-directories. All found unit-tests will be
 * added and executed.
 *
 * To run this suite from CLI: phpunit SuiteFull.php
 *
 * @package  test
 */
class SuiteFull extends PHPUnit_Framework_TestSuite
{
    /**
     * Suite factory.
     *
     * This function creates a PHPUnit test-suite,
     * scans the directory for test-cases,
     * adds all test-cases found and then returns
     * a test-suite containing all available tests.
     *
     * @access  public
     * @static
     * @return  SuiteFull
     */
    public static function suite()
    {
        $suite = new SuiteFull();
        $cwd = dirname(__FILE__);
        foreach (self::getFiles($cwd) as $file)
        {
            $suite->addTestFile($file);
        }
        return $suite;
    }

    /**
     * recursively list contents of a directory
     *
     * Returns a numeric list of all PHP-files found
     * in the directory and it's sub-directories.
     *
     * Each file is given with full path and file
     * name.
     *
     * @access  private
     * @static
     * @param   string  $directory  base directory
     * @param   array   &$files     used for recursion only
     * @return  array
     */
    private static function getFiles($directory, array &$files = array())
    {
        // read contents from directory
        if (is_dir($directory)) {
            foreach (scandir($directory) as $entry)
            {
                // skip entries '.' and '..'
                if ($entry[0] !== '.') {
                    $entry = $directory . DIRECTORY_SEPARATOR . $entry;
                    // recursion for sub-directories
                    if (is_dir($entry)) {
                        self::getFiles($entry, $files);
                    // add php-files to list
                    } elseif (preg_match('/test\.php$/i', $entry)) {
                        $files[] = $entry;
                    } else {
                        // ignore non-PHP files
                    }
                }
            }
            return $files;

        // target directory does not exist
        } else {
            // you might want to throw an exception here
            return array();
        }
    }
}
?>
Comment 6 Tomas Mysik 2009-07-17 09:10:21 UTC
> But I would suggest not to name the file "all-tests.php" [...]

Yes, I plan to name it NetBeansSuite.php (containing NetBeansSuite class).

To your script: please, next time attach it instead of copy & paste, thanks. I have a similar script, I will attach it, please review it (I used your PHPDoc for
class).
Comment 7 Tomas Mysik 2009-07-17 09:12:11 UTC
Created attachment 84876 [details]
NetBeansSuite
Comment 8 Tomas Mysik 2009-07-17 09:20:22 UTC
I would appreciate if you could try the script on Windows. Thanks.
Comment 9 marcusson 2009-07-17 19:49:41 UTC
I checked your code on windows and with a fresh version of PHP-Documentor.

There is an issue with your code. The file-matching is case-sensitive and will not find "*test.php". File-names are not
case-sensitive on Windows systems, so your rglob()-function should behave alike. This will improve the robustness of
your implementation.
Also old-school web-programmers have a tendency to lower-case all files, as this was a very common pitfall. I do this as
well and I know others who still stick with this convention, even though it might seem a little old-fashioned. In
addition: class-names are case-insensitive in PHP too! So a file-name "footest.php" actually matches a class called
"FooTest". Trying to define a class "FooTest" after declaration of a class "footest" would result in "Fatal error:
Cannot redeclare class FooTest".

This is also why I decided no to use glob() on my example. I choose to use preg_match because it allows case-insensitive
matching - even though I know that it makes the code look far more complicated.

PHPDocumentor reports the following errors:

Warning on line 11 - no @package tag was used in a DocBlock for class NetBeansSuite
Warning on line 39 - File "NetBeansSuite.php" has no page-level DocBlock, use @package in the first DocBlock to create one

So you missed the page-level DocBlock and the package definitions for both documentation blocks. Without that the
documentation of your file will be empty and your class will automatically be assigned to package "default".

In addition: you may not use the a-tag in your documentation. Use the PHPDoc-style element {@link } instead. I will
attach the generated documentation, so you may check it yourself.
Comment 10 marcusson 2009-07-17 19:51:41 UTC
Created attachment 84890 [details]
API docs generated by PHP-Documentor
Comment 11 Tomas Mysik 2009-07-20 08:59:59 UTC
Hi, thanks a lot for your feedback!

> The file-matching is case-sensitive and will not find "*test.php".

Yes, I have fixed that.

> [errors in PHPDoc]

I will fix that and push my changes later today so you can review it online.
Comment 12 Tomas Mysik 2009-07-20 10:54:47 UTC
Fixed, please verify. Thanks again for your cooperation.

Few notes:
- suite file should be well formatted
- suite file should not be in code coverage
- suite file should be well documented (using PhpDoc information)
- there's no fallback - if the suite file is "wrong" (syntax error e.g.), error message is displayed in Test Runner UI (so no fallback to run PHPUnit against
directory, it's not easy to do so)

http://hg.netbeans.org/web-main/rev/0fac9d961e46
http://hg.netbeans.org/web-main/rev/e10b2e18be8d
http://hg.netbeans.org/web-main/rev/d977cd4f4bb9
Comment 13 marcusson 2009-07-20 14:44:22 UTC
If reviewed the code changes. Looks good. Thank you for a job well done!

Just the PHPDoc comments still need a moment of your time - but you already mentioned that.

I saw that you included the project license at the file's doc-block. Good idea!
However: remember to add the PHPDoc package-tag also ;-)

In addition to your notes:

- suite file should be well formatted

That's a bit tricky, since the understanding, what "well formatted" means, depends on the author. I guess we should best
off, if we stick to the PEAR coding principles, which are most common.

On the other hand - it would be even better to use NetBeans build-in automated code formatting feature. It already uses
project specific formatting rules. This should at least ensure we use proper indentation.

I would suggest that everything beyond that point should be left to CodeSniffer, or similar tools, that check coding-styles.

- suite file should not be in code coverage

PHPUnit should take care of this, and - as far as I can tell - it does that.

- suite file should be well documented (using PhpDoc information)

Yes :-)

- there's no fallback

Yes.

However: if you feel inconvenient with this behavior, you could of course make the creation of the generic test-suite
optional. E.g. you could add an item "create default test-suite" to the PHPUnit menu, if you wish.
As long as the default test-suite is a project specific setting, you could decide to fall back to scanning the
directory, if the setting is empty.
Comment 14 Tomas Mysik 2009-07-20 15:10:17 UTC
> If reviewed the code changes. Looks good. Thank you for a job well done!

Thanks a lot for your help!

> Just the PHPDoc comments still need a moment of your time - but you already mentioned that.

What exactly do you mean? I have corrected the PHPDoc so now it should be final.

> However: remember to add the PHPDoc package-tag also ;-)

As written above, it is there.

> - suite file should be well formatted [...]

The main "problem" here is the opening curly bracket (on the new line vs. on the same line) - and this should be
fixed by calling internal reformating (it takes into account specific settings of the current project - I tried it and it
works for me).

> - suite file should not be in code coverage
> PHPUnit should take care of this, and - as far as I can tell - it does that.

Unfortunately, not true - as you can see in the comments, "@codeCoverage..." tags seem to be broken; I had to fix
it in NetBeans.

> However: if you feel inconvenient with this behavior, you could of course make the creation of the generic
> test-suite [...]

I know but this is more flexible for NetBeans users and it fixes this issue as well as your enhancement - right? But
this suite is used only for running _all_ the tests and not for one particular test... One test is still run using PHPUnit
directly - can we change this and use our own suite as well? It would be great, I would be able to show UI for test
results "on the fly" (now he have to wait for the XML log).
The only worry here is that users can change (or even break) this file... Also, any future changes to this file are a
bit tricky - they won't be applied for already existing files...
If really needed I can add some option for generating this suite but I would prefer not to do that.

Thanks for your cooperation!
Comment 15 Quality Engineering 2009-07-20 17:36:18 UTC
Integrated into 'main-golden', will be available in build *200907201401* on http://bits.netbeans.org/dev/nightly/ (upload may still be in progress)
Changeset: http://hg.netbeans.org/main-golden/rev/0fac9d961e46
User: Tomas Mysik <tmysik@netbeans.org>
Log: #167519: PHPUnit tests not executed if filename doesn't match class name
Comment 16 marcusson 2009-07-20 20:43:16 UTC
> What exactly do you mean? I have corrected the PHPDoc so now it should be final.

You are right. It was just me - I was checking your previous submission.
I've checked the final version. The result looks very good.

>The main "problem" here is the opening curly bracket (on the new line vs. on the same line) - and this should be
> fixed by calling internal reformating (it takes into account specific settings of the current project - I tried it
> and it works for me).

Confirmed and agreed.

> as you can see in the comments, "@codeCoverage..." tags seem to be broken;


I've noticed that all tags seem to be broken in the current release of PHPUnit. @test, @ignore and @DataProvider won't
work as well. I guess it's a bug in PHPUnit.

>> PHPUnit should take care of this, and - as far as I can tell - it does that.
> Unfortunately, not true [...] I had to fix it in NetBeans.

Actually the problem runs a little deeper. If you import external classes, like PEAR, it will report CodeCoverage for
these files, too! I've witnessed this behavior in CruiseControl/PHPUnderControl as well.

As far as I can see, the only way to fix this would be to check, whether the reported file is part of the tested project
or not.
However: if you wish to implement that, be sure to remember the new "ignore folder"-feature, that you posted earlier in
your blog (http://blogs.sun.com/netbeansphp/entry/ignored_folders_sure). I guess you would not want to create
coverage-reports on ignored folders as well ;-)

Maybe we should open another ticket for this issue. But I wouldn't call this a top-priority ;-)

> One test is still run using PHPUnit directly - can we change this and use our own suite as well?

Yes, you can change the suite to run just a single file of your choice. We would need to update the suite to implement this.

Should I look into this? I could make it react to a command-line argument, if you feel comfortable with that.

We could also make it run a single test-function instead of the whole file, if you wish.
This is a little more tricky. I will need to write a generic test-class, that implements the interface
PHPUnit_Framework_Test, takes the test's name as input and runs the test.

There is also a way to retrieve the result-object, so you could display a message in NetBeans directly. I haven't looked
into the details yet, but I found the appropriate function in the API-docs and since we use our own test-suite already,
it could be done. I could check the fine-print on how to implement this, if you wish.

By the way: did you know you may also define a test-suite and other parameters using XML?
You might want to have a look at this document: http://www.phpunit.de/manual/3.3/en/appendixes.configuration.html
I haven't tested it though. It might be uncommon to use XML here, but it's good to know that there is an alternative.
Comment 17 Tomas Mysik 2009-07-21 09:18:01 UTC
> [code coverage]
> Maybe we should open another ticket for this issue. But I wouldn't call this a top-priority ;-)

Please, do so. It should be easy to check the path of every file and skip non-project files. Thanks a lot.

> Yes, you can change the suite to run just a single file of your choice. We would need to update the suite to
> implement this.
> Should I look into this? I could make it react to a command-line argument, if you feel comfortable with that.

That would be great! It would be perfect to have it for Milestone 1, it means to implement it this or next week. But it
is not necessary. Please, report another issue for this. Thanks.

> There is also a way to retrieve the result-object, so you could display a message in NetBeans directly. I haven't
> looked into the details yet, but I found the appropriate function in the API-docs and since we use our own
> test-suite already, it could be done. I could check the fine-print on how to implement this, if you wish.

Yes, that would be great, again :) 

> [XML configuration]

I know about it but its capabilities are limited (you cannot attach listener there, for example). Moreover, this XML
configuration as well as bootstrap.php are meant to be updated/edited by user (this does not apply for
NetBeansSuite, any user changes should be avoided in this file). So, from these reasons, I would prefer to do
everything in "our" NetBeans suite, if possible.
Comment 18 marcusson 2009-07-21 10:49:40 UTC
I will look into this and file another issue. I will try to get it done by the end of this week, but I can't promise it
will be ready in time. Just keep fingers crossed.

I've checked the latest nightly-build and I'm a little afraid that people might not want a file called
"NetBeansSuite.php" to be auto-generated in their project sources.

This is because people may want to check in the basic test-suite to their repository. If so, they also might want to
avoid any kind of IDE-branding for files inside the repository.
Of course they could rename the file - but NetBeans would create it again. So they might end up being forced to add a
SVN:ignore to this folder and/or add a symlink. We should avoid that.

People might want to choose the name themselves as a project specific setting.

I guess you wouldn't want to add a "EclipseSuite.java" to your repository as well ;-)

I also noticed, that the doc-block "Suite factory ..." for function "suite()" is not indented properly. You may want to
fix that.
Comment 19 Tomas Mysik 2009-07-21 12:50:53 UTC
> [visible NetBeansSuite.php]

I'm afraid we cannot allow users NOT to generate this file - it must exist if we want to rely on it (as we _do_). I was
thinking about the proper location - we can generate it inside nbproject folder - then, it will stay hidden and so we
don't be afraid because of user changes... I plan to do that and it will be done soon (the final location will be
probably nbproject/private directory). It would help if we were able to add argument for folder/file that should be
run using this suite. Another question is about bootstrap.php and configuration.xml - these files are meant to be
changed by users and are generated directly in test directory...

> I guess you wouldn't want to add a "EclipseSuite.java" to your repository as well ;-)

Personally, I don't see any problem with just ignoring this file in my VCS. But I agree that some people can have
different opinion.

> I also noticed, that the doc-block "Suite factory ..." for function "suite()" is not indented properly. You may want to
> fix that.

Issue already filed (not project but editor area).