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

Changes to support phpunit 9.x from Debian for FRAMEWORK_6 branch #5

Merged
merged 1 commit into from
Dec 4, 2023

Conversation

Schlue
Copy link
Contributor

@Schlue Schlue commented Nov 28, 2023

@ralflang here the phpunit9.x changes for the FRAMEWORK6.0 branch.

Copy link
Member

@ralflang ralflang left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approved, with some comments. I will wait for the pipeline and then decide if I merge right now or do some touch up first.

@@ -28,9 +28,9 @@
* @link http://www.horde.org/apps/mnemo
*/
class Mnemo_TestCase
extends PHPUnit_Framework_TestCase
extends Horde_Test_Case
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not sure I agree with that move.

  1. We should use the namespaced tree Horde\Test\Case
  2. If it works with a bare Phpunit\Framrwork\Testcase, we should probably use that.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OTOH I can follow up on this separately. I don't want to delay / burden your otherwise uncontroversial fix to the unit tests

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi Ralf, the following is working for me:

use PHPUnit\Framework\TestCase;

class Mnemo_TestCase
extends TestCase

That would be solution 2. you prefer, or?

{
public function setUp()
public function setUp(): void
{
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, the return type must be added either way.

@ralflang ralflang merged commit e32e495 into horde:FRAMEWORK_6_0 Dec 4, 2023
0 of 6 checks passed
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