AtomParser_Parse_Test::test_parse_sets_handlers failing

[benniledl]

Hello,
this test was recently added and is failing for some reason.
Adding the debug flag public $debug = true; does not give a more detailed error message.
Changing the failing assertion so that it passes, reveals that the next assertions in this test also fail.

AtomParser_Parse_Test::test_parse_sets_handlers
end_ns() handler did not get called expected nr of times
Failed asserting that 2 is identical to 0.

/var/home/servertechnik_com/wp-tests/wp-env/tests/phpunit/tests/atomlib/AtomParser_Parse_Test.php:71

----

AtomParser_Parse_Test::test_parse_sets_handlers
cdata() handler did not get called expected nr of times
Failed asserting that 97 is identical to 57.
/var/home/servertechnik_com/wp-tests/wp-env/tests/phpunit/tests/atomlib/AtomParser_Parse_Test.php:72

----

AtomParser_Parse_Test::test_parse_sets_handlers
_default() handler did not get called expected nr of times
Failed asserting that 7 is identical to 2.
/var/home/servertechnik_com/wp-tests/wp-env/tests/phpunit/tests/atomlib/AtomParser_Parse_Test.php:73

​https://make.wordpress.org/hosting/test-results/r59073/
Domaintechnik
Failed PHP 8.3.11 mysql Ver 8.0.36-2 for Linux on x86_64 ((Debian))
Failed PHP 8.2.23 mysql Ver 8.0.36-2 for Linux on x86_64 ((Debian))
Failed PHP 8.1.29 mysql Ver 8.0.36-2 for Linux on x86_64 ((Debian))
Failed PHP 8.0.30 mysql Ver 8.0.36-2 for Linux on x86_64 ((Debian))
Failed PHP 7.4.33 mysql Ver 8.0.36-2 for Linux on x86_64 ((Debian))
Failed PHP 7.3.33 mysql Ver 8.0.36-2 for Linux on x86_64 ((Debian))
Failed PHP 7.2.34 mysql Ver 8.0.36-2 for Linux on x86_64 ((Debian))

[hellofromTonya]

@benniledl tests are failing after the introduction of this new AtomParser_Parse_Test test via [59062] / #62061. Moving this ticket into 6.7 for investigation.

I asked them to rollback the changes in the source code and rerun the tests:

I have manually rolled back these changes in the /src/ directory but the same happens.

So the issue is not due to the changes made in AtomParser::parse(). Marking this ticket then as Build/Test, as it seems to specific the tests and/or test's xml data.

Sharing more of Benni's debug findings:

I tried changing the assertion to 2 just to see if something else fails.

1) AtomParser_Parse_Test::test_parse_sets_handlers
cdata() handler did not get called expected nr of times
Failed asserting that 97 is identical to 57.
/var/home/servertechnik_com/wp-tests/wpenv/tests/phpunit/tests/atomlib/AtomParser_Parse_Test.php:73

then "correcting" above assertion, this is the next one that fails

1) AtomParser_Parse_Test::test_parse_sets_handlers
_default() handler did not get called expected nr of times
Failed asserting that 7 is identical to 2.
/var/home/servertechnik_com/wp-tests/wp-env/tests/phpunit/tests/atomlib/AtomParser_Parse_Test.php:74

so 3 assertions fail

[SergeyBiryukov]

The tests also fail for me locally when run in isolation, with a slightly different message though:

phpunit --filter AtomParser_Parse_Test
...
There was 1 failure:
1) AtomParser_Parse_Test::test_parse_sets_handlers
cdata() handler did not get called expected nr of times
Failed asserting that 60 is identical to 57.
S:\home\wordpress.test\develop-trunk\tests\phpunit\tests\atomlib\AtomParser_Parse_Test.php:72

[coquardcyr]

Hey @SergeyBiryukov,

I tried to run the command phpunit --filter AtomParser_Parse_Test on trunk.However I got no failure, am I missing something?

[SergeyBiryukov]

Replying to coquardcyr:

I tried to run the command phpunit --filter AtomParser_Parse_Test on trunk.However I got no failure, am I missing something?

It passes for me on Ubuntu, but fails on Windows. This could use some more investigation.

[jrf]

For the record: I created the test while running on Windows and now they are committed, they still pass for me on Windows too.

A brainfart just made think of line ending differences though, so I just to did a test run converting the fixture file line endings to CRLF and hey, presto, I can see the test failing, but just and only on the $atom->cdata_call_counter related assertion.

Changing that assertion to something like $this->assertGreaterThan( 56, $atom->cdata_call_counter, sprintf( $msg, 'cdata' ) ); makes it pass again for me and makes the complete test pass again too.

@benniledl @SergeyBiryukov Could you both check how you do your git check-out on Windows ? If you both use core.autocrlf = true instead of core.autocrlf = input, that would explain at least that difference.

Still doesn't explain the other differences though.

Having said that, it does confirm that - at least for this part - this is a pre-existing issue, which just never was discovered because there were no tests covering the code.

I imagine we may be able to do some line ending normalization before parsing the feed (or during). Then again, if the handlers deal with "empty" nodes correctly, we could also treat this as a test-only issue and change the relevant assertions from assertSame() to assertGreaterThan() as - after all - this test is just about verifying that the handlers get called, not about testing the functionality of the handlers.

But before we take any decision, I think it would be good to discover the underlying cause for the other failing tests, so let's start with checking the core.autocrlf setting and verifying that if that is set to input while on Windows, the tests pass. If they still don't pass, we need to dig deeper.

[benniledl]

I have checked, autocrlf is not set for me. But now that you say that reminds me, I have had a few issues with line ending characters in the past.
I tried setting core.autocrlf to input and running the test but it didnt change anything. I'm on Debian btw.

[benniledl]

Tests are still failing for me on php 8.3 - 7.4, they are now passing on 7.3 and 7.2, I have not changed anything on the environment besides now using the ​new multi php phpunit-test-runner
​https://make.wordpress.org/hosting/test-results/r59156/

[desrosj]

Hi @benniledl,

I am moving this to 6.7.1 to continue investigating.

I noticed that you're no longer reporting to the Host Tests, likely because you were just continuing to report failures. Could you confirm that you're still experiencing the issue?

[benniledl]

Hey @desrosj yes I am still having this issue.

[desrosj]

Because of a few bug reports opened since 6.7 was released, the Core team is evaluating the need for a short 6.7.1 cycle (possibly next week).

This issue was introduced during 6.7, but is not critical. I am going to punt, but test updates can be committed at any time.

[benniledl]

Just a note for todays 6.7.2 bug scrub, this problem is still the same: ​https://make.wordpress.org/hosting/test-results/r59527/

[jorbin]

Based off the suggestion from @jrf and testing by @benniledl reported in slack, We need a patch which does this:

Then again, if the handlers deal with "empty" nodes correctly, we could also treat this as a test-only issue and change the relevant assertions from assertSame() to assertGreaterThan() as - after all - this test is just about verifying that the handlers get called, not about testing the functionality of the handlers.

Trac ticket: https://core.trac.wordpress.org/ticket/62110

### Description

The AtomParser_Parse_Test::test_parse_sets_handlers test fails for the last three assertions. It was agreed that this is a tests-only issue, and a patch will be created to fix the test cases by using assertGreaterThan instead of assertSame. The test intends to verify that the handlers are being called, not to validate their functionality.

Additionally, some hosting environments are experiencing test failures due to the same issue. Relevant references can be found ​here.

### Screenshots

[jorbin]

In 59739:

Build/Test: Update assertions in AtomParser_Parse_Test::test_parse_sets_handlers

On some instances of Windows, the assertions seem to find additional nodes. As this test is just about verifying that the handlers get called, not about testing the functionality of the handlers, we can adjust the assertion to look for a minimum number of nodes rather than exact number.

Follow-up to [59062].

Props yogeshbhutkar, hellofromTonya, SergeyBiryukov, coquardcyr, jrf, benniledl, desrosj, jorbin.
Fixes #62110. See #62061.

[jorbin]

Reopening for backport consideration

[desrosj]

[59739] looks good for backporting!

[jorbin]

In 59745:

Build/Test: Update assertions in AtomParser_Parse_Test::test_parse_sets_handlers

On some instances of Windows, the assertions seem to find additional nodes. As this test is just about verifying that the handlers get called, not about testing the functionality of the handlers, we can adjust the assertion to look for a minimum number of nodes rather than exact number.

Follow-up to [59062].

Reviewed by desrosj.
Merges [59739] to the 6.7 branch.

Props yogeshbhutkar, hellofromTonya, SergeyBiryukov, coquardcyr, jrf, benniledl, desrosj, jorbin.
Fixes #62110. See #62061.