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

Switch to phpunit #17355

Merged
merged 15 commits into from
Jul 3, 2024
Merged

Conversation

trasher
Copy link
Contributor

@trasher trasher commented Jun 25, 2024

Q A
Bug fix? no
New feature? no
BC breaks? no
Deprecations? no
Tests pass? No :D

@cconard96
Copy link
Contributor

Adding these annotations to the PHPDoc of the Inventory test class fixes the outstanding test failure, but it is yet another workaround. Unfortunately adding the annotations in a parent class like GLPITestCase doesn't seem to work.

/**
 * @runTestsInSeparateProcesses
 * @preserveGlobalState disabled
 */

@trasher
Copy link
Contributor Author

trasher commented Jun 25, 2024

Adding these annotations to the PHPDoc of the Inventory test class fixes the outstanding test failure, but it is yet another workaround. Unfortunately adding the annotations in a parent class like GLPITestCase doesn't seem to work.

/**
 * @runTestsInSeparateProcesses
 * @preserveGlobalState disabled
 */

Thanks, it seems that works! I've tried similar annotations to fix other issues; but I did not try with this last remaining one... Annotating parent class won't work, it's stated in docs/issues I've read.
I'm not sure this is really a workaround; the issue is really dependent on tests (works well IRL , and works well with current atoum suite).

@trasher trasher force-pushed the feature/phpunit branch 2 times, most recently from 76ec40d to 4e2e67b Compare June 27, 2024 11:36
@cedric-anne
Copy link
Member

Adding these annotations to the PHPDoc of the Inventory test class fixes the outstanding test failure, but it is yet another workaround. Unfortunately adding the annotations in a parent class like GLPITestCase doesn't seem to work.

/**
 * @runTestsInSeparateProcesses
 * @preserveGlobalState disabled
 */

Thanks, it seems that works! I've tried similar annotations to fix other issues; but I did not try with this last remaining one... Annotating parent class won't work, it's stated in docs/issues I've read. I'm not sure this is really a workaround; the issue is really dependent on tests (works well IRL , and works well with current atoum suite).

Maybe adding the processIsolation="true" property to the XML configuration file would be safer (if it works). GLPI uses many static properties and it increases the conflict probability between tests.

Copy link
Member

@cedric-anne cedric-anne left a comment

Choose a reason for hiding this comment

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

I did not reviewed all the tests yet.

Do you plan to convert all the tests in a unique PR?

phpunit.xml.dist Show resolved Hide resolved
phpunit.xml.dist Show resolved Hide resolved
phpunit/DbTestCase.php Show resolved Hide resolved
phpunit/DbTestCase.php Show resolved Hide resolved
phpunit/GLPITestCase.php Outdated Show resolved Hide resolved
phpunit/InventoryTestCase.php Outdated Show resolved Hide resolved
phpunit/bootstrap.php Outdated Show resolved Hide resolved
phpunit/bootstrap.php Outdated Show resolved Hide resolved
phpunit.xml.dist Show resolved Hide resolved
@trasher
Copy link
Contributor Author

trasher commented Jul 1, 2024

Maybe adding the processIsolation="true" property to the XML configuration file would be safer (if it works). GLPI uses many static properties and it increases the conflict probability between tests.

As I said in internal channel, using process-isolation with phpunit 9.6 does not works. It does with phpunit >= 10.

@trasher
Copy link
Contributor Author

trasher commented Jul 1, 2024

I did not reviewed all the tests yet.

Do you plan to convert all the tests in a unique PR?

No, I tried in the past, this takes too much time, dealing with changes in existing tests is a nightmare.

Current PR moves existing atoum tests, so both can run at the same time. I'll open other PR's to migrate other tests later (in facts, there is already one other that is almost ready).

@trasher trasher marked this pull request as ready for review July 3, 2024 09:23
@cedric-anne cedric-anne merged commit 86a0655 into glpi-project:10.0/bugfixes Jul 3, 2024
13 checks passed
@trasher trasher deleted the feature/phpunit branch July 4, 2024 13:02
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.

3 participants