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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Repeating tests #5174

Closed
tanerkay opened this issue Feb 6, 2023 · 19 comments
Closed

Repeating tests #5174

tanerkay opened this issue Feb 6, 2023 · 19 comments

Comments

@tanerkay
Copy link

tanerkay commented Feb 6, 2023

Edit: it looks like this decision is being reverted 馃帀 thanks @jrfnl for bringing this to my attention. See #5718

Now that the --repeat option has been removed as of phpunit 10, I'm wondering if this feature is planned to be implemented in another way?

I tried without success to find any discussion regarding this change, and am curious as to why it has been removed. Is the onus on test runners or IDEs to provide the repeat functionality going forward? Or is the idea that developers should not be relying on the need for repeating tests?

Context: my use case for repeating tests is often when random data is involved, or in imperfect test suites where tests are not run in isolation, and database or application state between tests affects results. It helps identify, debug and improve these flaky tests.

Edit: as this decision seems unlikely to get reverted, I'm going to list some of the workarounds I've been using to get around this.

Workarounds:

  1. Using PHP's repeat configuration directive:
php -d repeat=100 vendor/bin/phpunit ...
Or in your IDE settings. For example, in PhpStorm:

image

  1. Use PestPHP if that is an option, where you can chain ->repeat($times) onto a test. Docs
@tanerkay tanerkay added the type/enhancement A new idea that should be implemented label Feb 6, 2023
@sebastianbergmann
Copy link
Owner

Now that the --repeat option has been removed as of phpunit 10, I'm wondering if this feature is planned to be implemented in another way?

Bringing back this functionality is not planned.

@sebastianbergmann sebastianbergmann removed the type/enhancement A new idea that should be implemented label Feb 6, 2023
@BinaryKitten
Copy link

@sebastianbergmann - I appreciate that the feature has been removed and isn't being planned to be brought back.

Is there any documentation around the decision for it's removal? - so far we've found the commit that removes it but would love to understand the root cause and where it came from so any further information as to the change would be highly desirable and welcomed.

@sebastianbergmann
Copy link
Owner

Is there any documentation around the decision for it's removal?

Unfortunately not, sorry.

@BinaryKitten
Copy link

Thank you anyway.

Appreciate the response as always.

@evgeek
Copy link

evgeek commented Feb 14, 2023

I sincerely hope that this functionality will return over time, because it is incredibly useful. In some cases, a test might only succeed 99% of the time, and --retry helped find the reasons why the remaining 1% of runs didn't work.

Maybe someone will be interested in the workaround I use:

trait MultipleRuns
{
    public function multipleRuns(): array
    {
        $result = [];
        for ($i = 0; $i < self::RUNS; $i++) {
            $result[] = [];
        }
        return $result;
    }
}

In test class:

  1. use MultipleRuns;
  2. protected const RUNS = X; (X for number of retries)
  3. @dataProvider multipleRuns in the phpdocs of the repeatable test

@sanmai
Copy link
Contributor

sanmai commented Apr 15, 2023

As a workaround, consider using a shell loop:

while php vendor/bin/phpunit tests/ExampleTest.php --stop-on-failure; do echo; done

Although this is not a replacement when you're looking to weed out interdependent tests.

@lphilps
Copy link

lphilps commented Jun 6, 2023

Very sad. I used this option all the time. I'm really going to miss it!

jrfnl added a commit to PHPCSStandards/PHPCSUtils that referenced this issue Jun 12, 2023
PHPUnit 10 makes significant changes to the configuration file.
While there is a `--migrate-configuration` option available in PHPUnit, that doesn't get us a well enough converted configuration file, so instead use a separate `phpunit10.xml.dist` file for running the tests on PHPUnit 10 with an optimal configuration.

Includes
* Adding separate scripts to the `composer.json` file to make this more obvious for contributors.
* Updating the GH Actions workflows to take the new configuration file into account when appropriate.
* Selectively not using the `--repeat` option, which was [removed without replacement](sebastianbergmann/phpunit#5174) in PHPUnit 10.
jrfnl added a commit to PHPCSStandards/PHPCSUtils that referenced this issue Jun 12, 2023
PHPUnit 10 makes significant changes to the configuration file.
While there is a `--migrate-configuration` option available in PHPUnit, that doesn't get us a well enough converted configuration file, so instead use a separate `phpunit10.xml.dist` file for running the tests on PHPUnit 10 with an optimal configuration.

Includes
* Adding separate scripts to the `composer.json` file to make this more obvious for contributors.
* Updating the GH Actions workflows to take the new configuration file into account when appropriate.
* Selectively not using the `--repeat` option, which was [removed without replacement](sebastianbergmann/phpunit#5174) in PHPUnit 10.
@lphilps
Copy link

lphilps commented Jun 19, 2023

@sebastianbergmann Could I respectfully ask you to re-consider the removal of this option? Now that it is is gone I have been made very aware of how often I used it, and how incredibly useful it was. It made debugging tests that failed very rarely so easy. There are workarounds but they are kludgy. And often for me running a single test with --repeat 100 for example, and looking at the percentage of test failure was key clue as to the source of the problem - and I have not found a workaround for that yet.

@paulpreibisch
Copy link

paulpreibisch commented Aug 3, 2023

I am also not happy about this - Here is the workaround in PhpStorm:
Edit configuration settings

In the interpreter options:
-d memory_limit=2G --repeat=100

@sfilzek
Copy link
Sponsor

sfilzek commented Aug 14, 2023

The --repeat option was so incredibly useful at tracking down strange edge cases. Some times a test would fail due to a strange combination of data from faker or similar.. running the test with --repeat --stop-on-failure was a great was of fuzzing this data and finding the one strange edge case which broke things.

I would love to see this functionality returned in a future release.

@mikebronner
Copy link

mikebronner commented Aug 22, 2023

Agreed, we used repeat all the time to weed out edge cases on tests with randomized data, as well. Using the shell loops as described above, or with a php argument, has the distinct disadvantage that the test suite is rebuilt from scratch each time, which is not fast. We spin up databases and seed them, then use that as a base database for each test, which improves speed of our suite by only having to spin up the database once.

@AJenbo
Copy link
Contributor

AJenbo commented Sep 27, 2023

Really sad to see this go as it was one of the more useful options 馃様

@Nutickets
Copy link

Feeling the burn without this feature 馃 It's a life saver for those gnarly 'only fail some of the time' tests.

@Rogierownage
Copy link

I, too, would like this feature returned, please.

@nozkok
Copy link

nozkok commented Jan 5, 2024

I can't comprehend why such a functional feature has been removed.

I've created two aliases for my case, and I wanted to share them with you as well.

alias rt='repeat_php_test'

repeat_php_test() {
  while true; do
    command_to_repeat="php artisan test $1"
    eval "$command_to_repeat"
  done
}

alias rtf='repeat_php_test_with_filter'

repeat_php_test_with_filter() {
  while true; do
    command_to_repeat="php artisan test --filter $1"
    eval "$command_to_repeat"
  done
}

You can add these aliases to your ~/.bashrc file.
Then you can use it just typing like this examples:

For all tests: rt
Some tests: rt tests/ExampleTest.php
Filtered tests: rtf someFilterKey

@arthurshafikov
Copy link

This was a useful feature to test out unstable test cases. I personally have found and fixed a lot of unstable tests with this. It has to be returned IMO.

@brianwozeniak
Copy link

Same for my team as well. We have thousands and thousands of tests and once in a blue moon a test fails, but then subsequently passes again. We typically used the --repeat option to help diagnose these sort of tests by running than hundreds of times to figure out if it is indeed a randomly failing test which lets us narrow in on the problem so that it can be addressed. Often times this will be used in conjunction with a debugger as well only triggering the breakpoint where it reaches the failed position. Very helpful for us.

Everyone I have talked to does not understand the decision to remove the feature, it was great to have for diagnosing, debugging, and resolving flaky tests that need to be improved.

@dschotman
Copy link

So I found this issue after trying to debug randomly failing tests and wanting to use the super useful --repeat option, but of course that didn't work.
I find it quite weird that this option got deleted without any explanation so far as I'm concerned the removal of the repeat option should either be properly explained or be reimplemented again.

As others said, there are some workarounds but they don't feel as good and integrated as it used to be.

So I ended up looking up the commit in which this useful feature was removed (442b9ab) and in my local files copy-paste back all the removed bits to make it work again. With one exception, in src/TextUI/TestRunner.php:

- $_suite = TestSuite::empty();
+ $_suite = TestSuite::empty($suite->name());

As long as I don't run any composer commands this should stay intact and usable for a while. Good enough for me to debug these few tests, but of course the best solution would have been that I didn't have to waste my time over this.

@jrfnl
Copy link
Contributor

jrfnl commented Mar 9, 2024

Heads up: #5718

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

No branches or pull requests