[tests-only] Make TemplatesTest more flexible #40059

phil-davis · 2022-05-09T05:32:03Z

Description

Unit tests in CI currently run with the owncloud-ci/php image based on Ubuntu 18.04.

On Ubuntu 20.04 some unit tests fail. See the related issue. They are failing in CI and locally on my Ubuntu 20.04 system.

This PR adjusts the 2 failing TemplatesTest tests. For some reason, on Ubuntu 20.04 (with whatever settings we have in CI and on my local system), the HTML output of the templates is "nicely formatted" with new-lines and indenting. That makes the test cases fail, because the test cases currently expect a particular exact formatting.

PHP unit test fails: https://drone.owncloud.com/owncloud/core/35632/9/8
This PR addresses items (3) and (4).

There were 4 failures:

1) Test\LargeFileHelperGetFileSizeTest::testGetFileSizeViaCurl with data set #0 ('/drone/src/tests/data/lorem.txt', 446)
Failed asserting that null is identical to 446.

/drone/src/tests/lib/LargeFileHelperGetFileSizeTest.php:53

2) Test\LargeFileHelperGetFileSizeTest::testGetFileSizeViaCurl with data set #1 ('/drone/src/tests/data/strängé...2).txt', 446)
Failed asserting that null is identical to 446.

/drone/src/tests/lib/LargeFileHelperGetFileSizeTest.php:53

3) Tests\Core\Templates\TemplatesTest::test403
Failed asserting that two strings are equal.
--- Expected
+++ Actual
@@ @@
 '<!DOCTYPE html PUBLIC "-//W3C//DTD HTML 4.0 Transitional//EN" "http://www.w3.org/TR/REC-html40/loose.dtd">\n
-<html><body><ul><li class="error">\n
-		Access forbidden<br><p class="hint"></p>\n
-</li></ul></body></html>\n
+<html><body><ul>\n
+<li class="error">\n
+		Access forbidden<br>\n
+		<p class="hint"></p>\n
+	</li>\n
+</ul></body></html>\n
 '

/drone/src/tests/lib/TestCase.php:510
/drone/src/tests/lib/TestCase.php:488
/drone/src/tests/Core/Templates/TemplatesTest.php:9

4) Tests\Core\Templates\TemplatesTest::test404
Failed asserting that two strings are equal.
--- Expected
+++ Actual
@@ @@
 '<!DOCTYPE html PUBLIC "-//W3C//DTD HTML 4.0 Transitional//EN" "http://www.w3.org/TR/REC-html40/loose.dtd">\n
-<html><body><ul><li class="error">\n
-			File not found<br><p class="hint">The specified document has not been found on the server.</p>\n
-<p class="hint"><a href="/owncloud/">You can click here to return to ownCloud.</a></p>\n
-		</li></ul></body></html>\n
+<html><body><ul>\n
+<li class="error">\n
+			File not found<br>\n
+			<p class="hint">The specified document has not been found on the server.</p>\n
+			<p class="hint"><a href="/owncloud/">You can click here to return to ownCloud.</a></p>\n
+		</li>\n
+	</ul></body></html>\n
 '

/drone/src/tests/lib/TestCase.php:510
/drone/src/tests/lib/TestCase.php:488
/drone/src/tests/Core/Templates/TemplatesTest.php:16
--

The changed test code "normalizes" the expected and actual HTML before comparing. That way the test will not be so brittle - it will work even when the formatting of the HTML is a bit different.

Related Issue

#38348

How Has This Been Tested?

CI in this PR (demonstrates that the test still passes on the Ubuntu 18.04 CI image)
Local unit test run on my Ubuntu 20.04 laptop - passes
CI of PR #40006 (demonstrates that these 2 tests pass on the Ubuntu 20.04 CI image)

Types of changes

Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Database schema changes (next release will require increase of minor version instead of patch)
Breaking change (fix or feature that would cause existing functionality to change)
Technical debt
Tests only (no source changes)

Checklist:

phil-davis · 2022-05-16T04:21:19Z

@jvillafanez @janackermann @JammingBen can someone review this please. IMO it should be an easy review - no change to real code, just making the unit test more flexible.

jvillafanez · 2022-05-16T10:21:04Z

Did you try checking with https://phpunit.readthedocs.io/en/latest/assertions.html#assertxmlstringequalsxmlstring ? I'm not sure if it will work, but it's worthy to try.

phil-davis · 2022-05-16T12:01:00Z

Did you try checking with https://phpunit.readthedocs.io/en/latest/assertions.html#assertxmlstringequalsxmlstring ? I'm not sure if it will work, but it's worthy to try.

I tried just now. assertXmlStringEqualsXmlString complains about the <br> element in 403.php and 404.php. If I make those <br/> then it parses the strings as XML OK. So I fixed those 2 files.

But then it complained about a lot of white-space differences. There is space that is "printable" (actually would be part of the text that could be rendered). In real life that white-space makes no visible difference when a browser renders it as HTML. But s XML, the PHP assert sees it as different.

To make it pass, I needed use my normalizeHTML function anyway. (or had do to that sort of white-space cleanup anyway)

I pushed a commit - @jvillafanez please review again.

sonarqubecloud · 2022-05-16T12:33:15Z

Kudos, SonarCloud Quality Gate passed!

0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells

No Coverage information
0.0% Duplication

jvillafanez · 2022-05-17T08:20:55Z

But then it complained about a lot of white-space differences. There is space that is "printable" (actually would be part of the text that could be rendered). In real life that white-space makes no visible difference when a browser renders it as HTML. But s XML, the PHP assert sees it as different.

I think that should be ok. I mean, if the text is different because there are white-spaces, then it different. We could considered it as a malformed document that should be fixed.

I'm having doubts with the normalizedHTML function because I don't think it covers all the cases, but if you're ok with it let's merge it.

phil-davis · 2022-05-17T08:51:12Z

I'm having doubts with the normalizedHTML function because I don't think it covers all the cases, but if you're ok with it let's merge it.

It is not intended to be a "general normalizer". It just does the "obvious" things that might be text white-space new-line differences which we know/decide that we don't care about for the purposes of the comparison in this test. If there was a need, then some full normalizer could be written (or find one on GitHub). But actually, we just want to allow some output variation to cope with the white-space differences in rendering that have been observed on different Ubuntu versions - presumably due to some HTML rendering utility version difference somewhere. And the difference is not so important, so we don't want to spend more time to exactly find out which version of which utility is causing this white-space difference.

Merging - this is a tests-only change, so we don't need to 'freeze" that, waiting for release-10.10.0

phil-davis added the QA:team label May 9, 2022

phil-davis self-assigned this May 9, 2022

phil-davis marked this pull request as ready for review May 9, 2022 07:10

phil-davis requested review from jvillafanez, JammingBen and AlexAndBear May 9, 2022 07:10

phil-davis mentioned this pull request May 10, 2022

Unit tests failing on Ubuntu 20.04 with PHP 7.4 #38348

Closed

Make TemplatesTest more flexible

ccd55f5

phil-davis force-pushed the flexible-TemplatesTest branch from fdded82 to ccd55f5 Compare May 16, 2022 04:19

Use assertXmlStringEqualsXmlString and fix <br/> elements

c616421

jvillafanez approved these changes May 17, 2022

View reviewed changes

phil-davis merged commit 9e6ed2e into master May 17, 2022

delete-merged-branch bot deleted the flexible-TemplatesTest branch May 17, 2022 08:51

jvillafanez mentioned this pull request Oct 25, 2022

Fix stream download release owncloud/files_primary_s3#618

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[tests-only] Make TemplatesTest more flexible #40059

[tests-only] Make TemplatesTest more flexible #40059

phil-davis commented May 9, 2022 •

edited

Loading

phil-davis commented May 16, 2022

jvillafanez commented May 16, 2022

phil-davis commented May 16, 2022

sonarqubecloud bot commented May 16, 2022

jvillafanez commented May 17, 2022

phil-davis commented May 17, 2022

[tests-only] Make TemplatesTest more flexible #40059

[tests-only] Make TemplatesTest more flexible #40059

Conversation

phil-davis commented May 9, 2022 • edited Loading

Description

Related Issue

How Has This Been Tested?

Types of changes

Checklist:

phil-davis commented May 16, 2022

jvillafanez commented May 16, 2022

phil-davis commented May 16, 2022

sonarqubecloud bot commented May 16, 2022

jvillafanez commented May 17, 2022

phil-davis commented May 17, 2022

phil-davis commented May 9, 2022 •

edited

Loading