-
Notifications
You must be signed in to change notification settings - Fork 7.8k
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
Generate and upload JUNIT test results on Windows CI #15897
Conversation
These are useful to get fine grained information about what was going on when the tests have been run, but won't clutter the build logs.
The issue is that it's hard to notice regardless. Maybe it would be better to specify a list of extensions that are expected to be tested, and error if they aren't. |
Many (most? all?) tests are skipped if a server is unavailable. These tests would need to be changed. But what should they report? We recently removed a probably unknown environment variable for mysqli tests, which would execute the tests to let them fail (instead of skipping them); that's not really helpful. Inspecting the junit results manually is tedious (although occassionally useful), but it's easy to write simple scripts like <?php
$doc = new DomDocument();
$doc->load("D:\Users\cmb\Downloads\junit.out.xml");
$xpath = new DOMXPath($doc);
$res = [];
foreach ($xpath->query("//testsuite[@skip > 10]") as $elt) {
$perc = $elt->getAttribute("skip") / $elt->getAttribute("tests") * 100;
if ($perc > 50) {
$res[$elt->getAttribute("name")] = $perc;
}
}
arsort($res);
print_r($res); which outputs:
Okay, snmp is a known issue now (#15896), fpm is non-Windows, pgsql tests are not executed (I know), but I'm surprised about ftp and standard/http. And the pdo tests are probably a false positive (due to redirects). One can have a look into these. |
Which is a little questionable in itself. Does it make sense to have hundreds of tests trying to ping a server that isn't available? IMO, it seems best if these tests didn't have a default configuration in the first place. And when the configuration is provided, not being able to connect should fail the test. Anyway, different issue.
It's probably possible to declare some threshold of tests that should be run per extension. E.g. if >50% are skipped, something fishy may be going on. Putting this logic in |
Maybe something like https://github.com/cmb69/php-ftw/blob/2865a4e059fb0b39f4f0d079c3754092e37f3972/.github/workflows/run-tests.ps1#L55-L61. That doesn't make the tests fail, if they are skipped, but avoids useless tests. Keeping dirs-to-test.txt up-to-date shouldn't be hard, but still is somewhat error prone.
That can make sense. Still, I feel that having the option to look into the junit results can be helpful occassionally ("has a certain test actually be run?"). See e.g. #15361 (comment); I had forced the test to fail, mostly to see that it actually had been executed.
Ah, nope, known issue. The servers are using |
Just to clarify: I don't object to re-adding JUNIT. I didn't find it useful, and time-consuming to look at. I would prefer if we could also generate some more useful statistics though, that don't require downloading and scanning a huge file. |
I haven't had the need to check JUNIT results for quite some time, and if we generate these files, we should do it for all CI jobs. I'm closing this PR for time reasons. |
These are useful to get fine grained information about what was going on when the tests have been run, but won't clutter the build logs.
Now I see that these have been deliberately removed via #14555, stating as reasons:
The former makes me sad. It's hard to notice that a complete test suite is skipped because a required server isn't properly set up (see e.g. #15896). I would be happy to have the JUNIT results available again, if only for Windows CI.