Use ImageAssert comparison, as MetadataExtentApiTests checksums differ based on font engine #5302

jodygarnett · 2021-01-04T19:05:11Z

This has been an outstanding issue for some months, ignoring these checks allows remaining tests to run. This commit has been isolated from #5260 as per @cmangeat request.

To quickly verify this change:

cd services
mvn test -Dtest=MetadataExtentApiTest

I guessed at a tolerance, would love confirmation on:

macOS OpenJDK and OracleJDK
linux
windows

image comparison

I am going to take a moment to see if I can do an image compare and have a stable build across platforms.

To run test with interactive review of failures:

mvn clean install -DMetadataExtentApiTest.save.png=true -Dorg.geotools.image.test.interactive=true -Dtest=MetadataExtentApiTest -Djava.awt.headless=false

When running without a reference file, there is an option to save a new reference file:

Images that are within the provided tolerance are accepted
Images outside the provided tolerance are rejected
When running with -Dorg.geotools.image.test.interactive=true any failures can be visually inspected, with an option to update the reference image.

This has been an outstanding issue for some months... This check allows remaining tests to run.

This allows the use of a tollarance to account for font differences between platforms.

jodygarnett · 2021-01-05T05:29:58Z

Travis failed to download a jar (unrelated to this PR?):

/home/travis/.m2/repository/org/geotools/gt-coverage/23.0/gt-coverage-23.0.jar; error in opening zip file

This file is available so travis is just having trouble.

⚠️ Note travis is shutting down shortly, any plans to migrate to github?

jodygarnett · 2021-01-05T15:05:38Z

services/src/test/java/org/fao/geonet/api/records/extent/MetadataExtentApiTest.java

+        assertImage(
+            REFERENCE+"lastModifiedModified.png",
+            reponseBuffer,
+            37);


@josegar74 this is an example of a tolerance that I guessed at to account for platform differences.

If you want you can reduce this to zero and we can get an actual difference between linux and macOS.

jodygarnett · 2021-01-05T15:05:58Z

services/src/test/java/org/fao/geonet/api/records/extent/MetadataExtentApiTest.java

    }

    @Test
    public void lastModifiedNotModified() throws Exception {
+        final boolean OS_NAME_MAC_OS = System.getProperty("os.name").toLowerCase().startsWith("mac os");
+        if( OS_NAME_MAC_OS ) return; // skip on macOS


This test is being skipped still as it does not work for me on macOS.

@josegar74 reported this test failing on linux:

[ERROR] org.fao.geonet.api.records.extent.MetadataExtentApiTest.lastModifiedModified Time elapsed: 12.078 s <<< FAILURE! java.lang.AssertionError: Range for response status value 304 expected:<SUCCESSFUL> but was:<REDIRECTION> at org.fao.geonet.api.records.extent.MetadataExtentApiTest.lastModifiedModified(MetadataExtentApiTest.java:166)

Since it is returning a result I thought I would collect the image, and keep the test enabled:

These tests are producing different results across platforms resulting in developers skipping tests

jodygarnett · 2021-01-06T01:13:28Z

Revised threshold based on ubuntu results:

[ERROR]   MetadataExtentApiTest.getOneRecordExtentAsImage:135->assertImage:414 Images are visibly different, found 521 different pixels, against a threshold of 37

I would still love a visual from this environment to see if is just font differences, or if the "no protocol setting" text is specific to macOS.

cmangeat · 2021-01-06T08:30:43Z

Revised threshold based on ubuntu results:
[ERROR]   MetadataExtentApiTest.getOneRecordExtentAsImage:135->assertImage:414 Images are visibly different, found 521 different pixels, against a threshold of 37
I would still love a visual from this environment to see if is just font differences, or if the "no protocol setting" text is specific to macOS.

(What do you use as jdk with osx ? If oracle, is it possible to test with openjdk ?)

jodygarnett · 2021-01-06T14:42:02Z

(What do you use as jdk with osx ? If oracle, is it possible to test with openjdk ?)

See description at the top of the PR, I tested with oracle and openjdk.

cmangeat · 2021-01-06T14:55:51Z

services/src/test/java/org/fao/geonet/api/records/extent/MetadataExtentApiTest.java

    }

-    @Test
+    @Ignore


cf.

core-geonetwork/services/src/test/java/org/fao/geonet/api/records/extent/MetadataExtentApiTest.java

Line 371 in 0eba538

calendar.set(2015, Calendar.OCTOBER, 21, 07, 28, 0);

last modification date is "2015, Calendar.OCTOBER, 21, 07, 28, 0"

version available at browser side date is "Wed, 21 Oct 2015 07:29:00 UTC"

response should be "not modified" ?

If the test is showing a new bug it can be addressed in a separate PR. I do wonder if the failure in this case is timezone sensitive ...

If service return "not modified" it is not a bug. Otherwise, the test should be deleted.

cmangeat · 2021-01-06T14:59:53Z

services/src/test/java/org/fao/geonet/api/records/extent/MetadataExtentApiTest.java

    }

-    @Test
+    @Ignore


In fact, as test purpose is about checking for "If-Modified-Since" and cache management, checking that returned image fits is not really important, but it anyway should ?

It was testing the returned image before using a checksum ... if the image is not of interest we can remove the check :)

I don't know why the test is failing with mac os. Checking that image retrieved from cache is the good one can make sense. Otherwise the test should be removed.

I think it is failing based on timezone, not macOS.

cmangeat · 2021-01-06T15:01:43Z

Great, proud to know there is an image compare somewhere !

jodygarnett · 2021-01-07T16:00:22Z

@juanluisrp was able to visually confirm the results, so I think this is good to go.

juanluisrp · 2021-01-07T16:02:34Z

linux.zip
Images generated using Lubuntu 20.04.1 with openjdk 8. I can't find any appreciable diff.

jodygarnett · 2021-01-11T06:56:05Z

linux.zip
Images generated using Lubuntu 20.04.1 with openjdk 8. I can't find any appreciable diff.

Thanks @juanluisrp - your download totally confirmed my suspicion: the text in the images show a difference between "expected" and "actual" in each case. The text shows up as white, so you need to show this against a dark background to see the difference!

Expected (from macOS):

Actual (in this case from linux):

This can be alleviated by including a specific font in the geonetwork war and making use of it when drawing error messages such as this on the returned image.

jodygarnett added 2 commits January 4, 2021 11:01

Ignore MetadataExtentApiTests on macOS as file checksums differ

fcf0132

This has been an outstanding issue for some months... This check allows remaining tests to run.

Use ImageAssert to verify expected output

5181c44

This allows the use of a tollarance to account for font differences between platforms.

josegar74 requested a review from cmangeat January 5, 2021 08:05

jodygarnett changed the title ~~Ignore MetadataExtentApiTests on macOS as file checksums differ~~ Use ImageAssert comparison, as MetadataExtentApiTests checksums differ based on font engine Jan 5, 2021

jodygarnett commented Jan 5, 2021

View reviewed changes

jodygarnett force-pushed the image_compare_fix branch from c009a04 to f5883c7 Compare January 5, 2021 16:46

Ignore lastModifiedNotModified and lastModifiedModified

0eba538

These tests are producing different results across platforms resulting in developers skipping tests

jodygarnett force-pushed the image_compare_fix branch from f5883c7 to 0eba538 Compare January 6, 2021 01:10

cmangeat reviewed Jan 6, 2021

View reviewed changes

josegar74 approved these changes Jan 8, 2021

View reviewed changes

josegar74 merged commit f33c500 into geonetwork:master Jan 8, 2021

jodygarnett mentioned this pull request Jan 11, 2021

MetaDataExtentApiTests tests of last modified #5324

Closed

jodygarnett deleted the image_compare_fix branch January 14, 2021 19:44

juanluisrp mentioned this pull request Jan 18, 2022

Index all atom links #6118

Open

juanluisrp mentioned this pull request Apr 25, 2022

Fix test errors #6265

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Use ImageAssert comparison, as MetadataExtentApiTests checksums differ based on font engine #5302

Use ImageAssert comparison, as MetadataExtentApiTests checksums differ based on font engine #5302

jodygarnett commented Jan 4, 2021 •

edited

jodygarnett commented Jan 5, 2021 •

edited

jodygarnett Jan 5, 2021

jodygarnett Jan 5, 2021 •

edited

jodygarnett Jan 5, 2021

jodygarnett commented Jan 6, 2021

cmangeat commented Jan 6, 2021

jodygarnett commented Jan 6, 2021

cmangeat Jan 6, 2021 •

edited

jodygarnett Jan 6, 2021

cmangeat Jan 7, 2021

cmangeat Jan 6, 2021

jodygarnett Jan 6, 2021

cmangeat Jan 7, 2021

jodygarnett Jan 7, 2021

cmangeat commented Jan 6, 2021

jodygarnett commented Jan 7, 2021

juanluisrp commented Jan 7, 2021

jodygarnett commented Jan 11, 2021 •

edited

                   }
-                  @Test
+                  @Ignore

Use ImageAssert comparison, as MetadataExtentApiTests checksums differ based on font engine #5302

Use ImageAssert comparison, as MetadataExtentApiTests checksums differ based on font engine #5302

Conversation

jodygarnett commented Jan 4, 2021 • edited

image comparison

jodygarnett commented Jan 5, 2021 • edited

Choose a reason for hiding this comment

jodygarnett Jan 5, 2021 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jodygarnett commented Jan 6, 2021

cmangeat commented Jan 6, 2021

jodygarnett commented Jan 6, 2021

cmangeat Jan 6, 2021 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cmangeat commented Jan 6, 2021

jodygarnett commented Jan 7, 2021

juanluisrp commented Jan 7, 2021

jodygarnett commented Jan 11, 2021 • edited

jodygarnett commented Jan 4, 2021 •

edited

jodygarnett commented Jan 5, 2021 •

edited

jodygarnett Jan 5, 2021 •

edited

cmangeat Jan 6, 2021 •

edited

jodygarnett commented Jan 11, 2021 •

edited