Fix for PHPUnit 10 #5790 #5853

junichi11 · 2023-04-20T05:19:43Z

PHPunit 10 incompatibility #5790
Add PhpUnitVersion
Don't use NetBeansSuite.php with PHPUnit 10 because suite() method is invoked no longer and we have to extend TestCase
See: sebastianbergmann/phpunit@36354a9

- apache#5790 - Add `PhpUnitVersion` - Don't use `NetBeansSuite.php` with PHPUnit 10 because `suite()` method is invoked no longer and we have to extend `TestCase` - See: sebastianbergmann/phpunit@36354a9

junichi11 · 2023-04-20T05:25:20Z

The suite class file is checked whether it is a subclass of TestCase:
https://github.com/sebastianbergmann/phpunit/blob/da707f3748773fbffd20289ae039e5e667d6b3a6/src/Runner/TestSuiteLoader.php#L42-L80

junichi11 · 2023-04-20T05:26:54Z

@tmysik What do you think? Any ideas?

php/php.phpunit/src/org/netbeans/modules/php/phpunit/ui/customizer/CustomizerPhpUnit.java

tmysik · 2023-04-20T08:24:37Z

Overall looks good to me. Here are my notes:

cannot we use the PHPUnit 10 way also for older versions of PHPUnit? Maybe we could avoid the PHPUnit version selection at all?
cannot we detect what version of PHPUnit is used? I mean, some cheap way, like existence of some file inside PHPUnit directory (not so nice but could work)
cannot we improve error message so user knows what to do if the running of the tests does not work? Something like "Verify PHPUnit version in the Project Properties dialog"

Thanks!

php/php.phpunit/src/org/netbeans/modules/php/phpunit/commands/PhpUnit.java

junichi11 · 2023-04-20T10:37:14Z

cannot we use the PHPUnit 10 way also for older versions of PHPUnit? Maybe we could avoid the PHPUnit version selection at all?

Maybe, PHPUnit 9 works the same way (for a single file and directory). However, I'm not sure about the others...

cannot we detect what version of PHPUnit is used? I mean, some cheap way, like existence of some file inside PHPUnit directory (not so nice but could work)

cannot we improve error message so user knows what to do if the running of the tests does not work? Something like "Verify PHPUnit version in the Project Properties dialog"

I'll try thinking whether we can run unit test without selecting the version :)
Thanks!

junichi11 · 2023-04-22T05:43:30Z

@tmysik We can detect the version via phpunit --version. What do you think?

I have no ideas about multiple files yet...

tmysik · 2023-04-22T08:14:47Z

@junichi11 The ideal solution, of course 👍

php/php.phpunit/src/org/netbeans/modules/php/phpunit/PhpUnitVersion.java

tmysik · 2023-04-22T10:20:59Z

@junichi11 Looks good to me. However, we still need to solve situation when several files should be tested (e.g. user selects more files, or re-run the failed tests).

junichi11 · 2023-04-22T10:52:00Z

@tmysik

However, we still need to solve situation when several files should be tested (e.g. user selects more files, or re-run the failed tests).

Yes. I'll submit that problem as a new issue after this is merged. (I have no ideas although I investigated a bit...)
Thank you for your review!

tmysik · 2023-04-22T10:54:09Z

@junichi11 You are welcome!

junichi11 · 2023-04-22T11:30:01Z

I'll click the "Ready for review" button to merge this after CI passes. (This PR is merged by the release team.)

KacerCZ · 2023-04-22T15:47:52Z

I tried to find way how to pass multiple files to PHPUnit, but there is no easy way.

Possible solution could be inspired by https://github.com/infection/infection.
It manipulates XML configuration for PHPUnit to suit its needs and creates temporary XML file.
This file is then passed to PHPUnit with -c/--configuration parameter.
See

Proposal

NetBeans creates temporary XML file
- if config file exists (specified in dialog configuration or default file name) then it copies its content
- relative paths in new file needs to be updated for new location
- if config file does not exist it creates new one
create its own test suite with unique name (https://docs.phpunit.de/en/10.1/configuration.html#the-testsuite-element)
add files/directories to the suite
run PHPUnit with parameters --configuration and --testsuite (https://docs.phpunit.de/en/10.1/textui.html) to test selected files

What do you think?

tmysik · 2023-04-22T15:57:02Z

@KacerCZ

Thanks for looking into it! I was thinking about this solution also for the original solution (instead of NetBeansSuite.php), however, it cannot work in cases when the user already has any existing PHPUnit configuration. Or am I missing something?

cc @junichi11

tmysik · 2023-04-22T15:58:48Z

@KacerCZ

Let me improve my comment - I meant, can we always "extend" properly any existing configuration provided by the user? I am not so sure...

KacerCZ · 2023-04-22T16:10:41Z

Existing configuration file must be copied to temporary file and modified - add test suite and update relative paths.

Configuration file can be configured in NetBeans or PHPUnit is looking for phpunit.xml, phpunit.dist.xml or phpunit.xml.dist in working directory (root of project).

tmysik · 2023-04-22T16:30:56Z

Cannot the existing configuration contain some suite already? I mean, can we always update it? Sorry if it is clear and my questions are unnecessary 🙈

KacerCZ · 2023-04-22T17:17:41Z

@tmysik No need to apologize.
Configuration file can contain one or more suites already defined.
Idea is to add new test suite definition with and run this new suite.

Name of the suite could be something like "NetBeansTempSuite" with MD5 hash created from pathnames passed from Netbeans to prevent name collision with already existing suite.

tmysik · 2023-04-22T17:22:11Z

Ha, if we can add a suite, then it seems to me to be the best way to go. Thanks for clarification!

junichi11 · 2023-04-22T22:15:07Z

Thanks for investigating it. I'll submit that problem as a new issue later.

junichi11 · 2023-04-22T23:28:10Z

I noticed that only one file is run with also PHPUnit 9 when we select several files. (I assumed that we can do it, sorry.)

Select two files (Tested files)
Run > Test files

Am I wrong something?

Rerun failed: tests are run with --filter param

tmysik · 2023-04-23T15:33:02Z

@junichi11

I can see 2 files were run, no?

junichi11 · 2023-04-23T23:08:11Z

@tmysik I should have written what I ran about that image, sorry. I selected the directory to show failed results for 2 files. I'm not sure how to run the selected 2 files actually. So, I've looked into it a bit.

netbeans/php/php.project/src/org/netbeans/modules/php/project/ui/actions/support/CommandUtils.java

Lines 299 to 309 in e65310a

    
               /** 
        
                * Get a <b>valid</b> {@link FileObject} for context or selected nodes. 
        
                * Return <code>null</code> if any {@link FileObject} is invalid. 
        
                * @param context context to search in. 
        
                * @return a <b>valid</b> {@link FileObject} for context or selected nodes or <code>null</code>. 
        
                * @see #fileForContextOrSelectedNodes(Lookup, FileObject) 
        
                */ 
        
               public static FileObject fileForContextOrSelectedNodes(Lookup context) { 
        
                   FileObject[] files = filesForContextOrSelectedNodes(context); 
        
                   return files.length > 0 ? files[0] : null; 
        
               }

It seems that the first item is returned when we select several files.

netbeans/php/php.project/src/org/netbeans/modules/php/project/ui/actions/support/ConfigActionTest.java

Lines 172 to 179 in e65310a

    
           @Override 
        
           public void runFile(final Lookup context) { 
        
               FileObject fileObj = CommandUtils.fileForContextOrSelectedNodes(context); 
        
               assert fileObj != null : "Fileobject not found for context: " + context; 
        
               TestRunInfo testRunInfo = getTestRunInfoForFile(fileObj, false); 
        
               assert testRunInfo != null; 
        
               run(testRunInfo); 
        
           }

tmysik · 2023-04-24T05:13:38Z

Uff :) Sorry, it has been a long time... Cannot we call filesForContextOrSelectedNodes()? But maybe the current behavior is "good enough" and running tests in a folder is what users do? Anyway, thanks for looking into it.

junichi11 · 2023-04-24T06:21:14Z

Cannot we call filesForContextOrSelectedNodes()?

We should be able to do it. Then, we have to fix getTestRunInfoForFile() or add getTestRunInfoForFiles().

But maybe the current behavior is "good enough"

Yes, I also think so :)

and running tests in a folder is what users do?

I'm not sure... but maybe, usage is the following via CLI, so users didn't report anything about running several test files, I guess.

Usage:
  phpunit [options] UnitTest.php
  phpunit [options] <directory>

It doesn't seem a big problem for users (I think we need not hurry) because there is no report from users for a long time.
At least, PHPUnit 10 should work fine with this PR if there are no wrong fixes in my changes :)

tmysik · 2023-04-24T11:29:31Z

I totally agree, not a big problem definitely.

BTW one more use-case is, if I remember it correctly 🙂, to rerun only failed tests. But that cannot work even now so all fine 👍 Let's create a ticket for it and merge this PR if there are no objections.

Thanks!

junichi11 · 2023-04-24T11:48:57Z

@tmysik I'll do that later :) This PR will be merged by the release team because I would like to include this in NB18. (We must not do it.) : https://lists.apache.org/thread/s3hnjtmr6t7dkyq1ofn4hdqmd0qh2d11
Thank you!

neilcsmith-net · 2023-04-24T11:55:29Z

This PR will be merged by the release team because I would like to include this in NB18.

Don't worry, I've been following the conversation! 😄

I know it's been moved out of draft, but wasn't 100% sure from the ongoing conversation if it is ready to merge. Please confirm, and I'll merge by tomorrow in time for 18-rc2.

junichi11 · 2023-04-24T11:59:53Z

@neilcsmith-net Thank you!

junichi11 · 2023-04-25T01:58:45Z

Created the issue: #5880

Fix for PHPUnit 10 apache#5790

e18e99b

- apache#5790 - Add `PhpUnitVersion` - Don't use `NetBeansSuite.php` with PHPUnit 10 because `suite()` method is invoked no longer and we have to extend `TestCase` - See: sebastianbergmann/phpunit@36354a9

junichi11 added the PHP [ci] enable extra PHP tests (php/php.editor) label Apr 20, 2023

junichi11 linked an issue Apr 20, 2023 that may be closed by this pull request

PHPunit 10 incompatibility #5790

Closed

junichi11 requested a review from tmysik April 20, 2023 05:27

lbownik reviewed Apr 20, 2023

View reviewed changes

php/php.phpunit/src/org/netbeans/modules/php/phpunit/ui/customizer/CustomizerPhpUnit.java Show resolved Hide resolved

tmysik approved these changes Apr 20, 2023

View reviewed changes

tmysik reviewed Apr 20, 2023

View reviewed changes

php/php.phpunit/src/org/netbeans/modules/php/phpunit/commands/PhpUnit.java Outdated Show resolved Hide resolved

tmysik reviewed Apr 20, 2023

View reviewed changes

php/php.phpunit/src/org/netbeans/modules/php/phpunit/commands/PhpUnit.java Show resolved Hide resolved

mbien added this to the NB18 milestone Apr 20, 2023

junichi11 force-pushed the php-gh-5790-phpunit10 branch from 01a07db to 72e8d5b Compare April 22, 2023 06:11

junichi11 force-pushed the php-gh-5790-phpunit10 branch from 72e8d5b to bcfb8f7 Compare April 22, 2023 10:03

tmysik reviewed Apr 22, 2023

View reviewed changes

php/php.phpunit/src/org/netbeans/modules/php/phpunit/PhpUnitVersion.java Show resolved Hide resolved

Detect the PHPUnit version

6ac8eac

junichi11 force-pushed the php-gh-5790-phpunit10 branch from bcfb8f7 to 6ac8eac Compare April 22, 2023 11:22

junichi11 marked this pull request as ready for review April 22, 2023 22:15

neilcsmith-net merged commit 92080c7 into apache:delivery Apr 24, 2023

junichi11 deleted the php-gh-5790-phpunit10 branch April 25, 2023 01:33

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix for PHPUnit 10 #5790 #5853

Fix for PHPUnit 10 #5790 #5853

junichi11 commented Apr 20, 2023

junichi11 commented Apr 20, 2023

junichi11 commented Apr 20, 2023

tmysik commented Apr 20, 2023 •

edited

Loading

junichi11 commented Apr 20, 2023

junichi11 commented Apr 22, 2023 •

edited

Loading

tmysik commented Apr 22, 2023

tmysik commented Apr 22, 2023

junichi11 commented Apr 22, 2023 •

edited

Loading

tmysik commented Apr 22, 2023

junichi11 commented Apr 22, 2023

KacerCZ commented Apr 22, 2023

tmysik commented Apr 22, 2023

tmysik commented Apr 22, 2023

KacerCZ commented Apr 22, 2023

tmysik commented Apr 22, 2023

KacerCZ commented Apr 22, 2023

tmysik commented Apr 22, 2023

junichi11 commented Apr 22, 2023

junichi11 commented Apr 22, 2023 •

edited

Loading

tmysik commented Apr 23, 2023

junichi11 commented Apr 23, 2023 •

edited

Loading

tmysik commented Apr 24, 2023

junichi11 commented Apr 24, 2023 •

edited

Loading

tmysik commented Apr 24, 2023

junichi11 commented Apr 24, 2023

neilcsmith-net commented Apr 24, 2023

junichi11 commented Apr 24, 2023

junichi11 commented Apr 25, 2023

Fix for PHPUnit 10 #5790 #5853

Fix for PHPUnit 10 #5790 #5853

Conversation

junichi11 commented Apr 20, 2023

junichi11 commented Apr 20, 2023

junichi11 commented Apr 20, 2023

tmysik commented Apr 20, 2023 • edited Loading

junichi11 commented Apr 20, 2023

junichi11 commented Apr 22, 2023 • edited Loading

tmysik commented Apr 22, 2023

tmysik commented Apr 22, 2023

junichi11 commented Apr 22, 2023 • edited Loading

tmysik commented Apr 22, 2023

junichi11 commented Apr 22, 2023

KacerCZ commented Apr 22, 2023

tmysik commented Apr 22, 2023

tmysik commented Apr 22, 2023

KacerCZ commented Apr 22, 2023

tmysik commented Apr 22, 2023

KacerCZ commented Apr 22, 2023

tmysik commented Apr 22, 2023

junichi11 commented Apr 22, 2023

junichi11 commented Apr 22, 2023 • edited Loading

tmysik commented Apr 23, 2023

junichi11 commented Apr 23, 2023 • edited Loading

tmysik commented Apr 24, 2023

junichi11 commented Apr 24, 2023 • edited Loading

tmysik commented Apr 24, 2023

junichi11 commented Apr 24, 2023

neilcsmith-net commented Apr 24, 2023

junichi11 commented Apr 24, 2023

junichi11 commented Apr 25, 2023

tmysik commented Apr 20, 2023 •

edited

Loading

junichi11 commented Apr 22, 2023 •

edited

Loading

junichi11 commented Apr 22, 2023 •

edited

Loading

junichi11 commented Apr 22, 2023 •

edited

Loading

junichi11 commented Apr 23, 2023 •

edited

Loading

junichi11 commented Apr 24, 2023 •

edited

Loading