adobe / adobe/aem-core-wcm-components

[PageImpl] Non-deterministic behavior or HashSet might fail tests in PageImplTest

Open
#2,893 0 comments 0 reactions 0 assignees View on GitHub
Dominant language
Java
Stars
795
Forks
775
Avg merge
3d 22h
Merged PRs (30d)
8

Description

This issue is a copy of https://github.com/adobe/aem-core-wcm-components/issues/2612#issue-1975315173

## Bug Report

**Current Behavior**
* While testing the PageImpl, the contents of Page are mocked using MockPage from the dependency io.wcm.testing.aem-mock.junit5.
* The mock values for Page is loaded from the following files based on the version of the PageImplTest, where the tags are in the order [one, two, three].
- `src/test/resources/page/test-content.json`
- `src/test/resources/page/v2/test-content.json`
* `src/test/resources/page/v3/test-content.json`
- The mocking occurs after instantiation of PageImpl at
https://github.com/adobe/aem-core-wcm-components/blob/a517a37a43007856988d02b84cb3042c62877dd8/bundles/core/src/main/java/com/adobe/cq/wcm/core/components/internal/models/v1/PageImpl.java#L150-L159

- However, while mocking the Tags of PageImpl at MockTagManager:380 (screenshots provided below), the values of the tags are converted from the loaded resource into a HashSet.
- The official [Javadoc](https://docs.oracle.com/javase/8/docs/api/java/util/HashSet.html) mentions that HashSet does not guarantee that the order will be maintained over time.
- This causes issues where the order of tags of tags is expected to be in a specific order.
- Specifically, the following tests expects the order of the tags after exporting to JSON to be in the order ["three", "one", "two"]
- com.adobe.cq.wcm.core.components.internal.models.v1.PageImplTest.testPage
- com.adobe.cq.wcm.core.components.internal.models.v2.PageImplTest.testPage
- com.adobe.cq.wcm.core.components.internal.models.v2.PageImplTest.testPageWithDeprecatedCaconfig
- com.adobe.cq.wcm.core.components.internal.models.v3.PageImplTest.testPage
- com.adobe.cq.wcm.core.components.internal.models.v3.PageImplTest.testPageWithDeprecatedCaconfig

**Steps to reproduce**
- The above mentioned issue is found using the [NonDex (https://github.com/TestingResearchIllinois/NonDex) tool, which shuffles the order of HashSet.

```
# Clone the Repo
https://github.com/adobe/aem-core-wcm-components.git

# Compile the module byte-buddy-dep
mvn clean install -pl bundles/core -am -DskipTests

# (Optional) Run the unit test
mvn -pl bundles/core test -Dtest=

# Run the unit test using NonDex
mvn -pl bundles/core edu.illinois:nondex-maven-plugin:2.1.1:nondex -Dtest=
```

- Following the above steps for the mentioned tests will produce the following result:
```
[INFO] Results:
[INFO]
[ERROR] Failures:
[ERROR] PageImplTest.testPage:109->testPage:142 \
expected: <{"id":"page-bb590de134","designPath":"/etc/designs/mysite","title":"Templated Page","description":"Description","brandSlug":"Brand Slug","lastModifiedDate":1453282416000,"templateName":"product-page","appResourcesPath":"/core/etc.clientlibs/wcm/core/page/clientlibs/favicon/resources","htmlPageItems":[{"element":"link","location":"header","attributes":{"href":"/_theme/theme.css","rel":"preload","as":"style"}},{"element":"script","location":"footer","attributes":{"async":true,"defer":false,"src":"/_theme/theme.js"}},{"element":"meta","location":"header","attributes":{"charset":"UTF-8"}}],"cssClassNames":"class1 class2","language":"en-GB",":itemsOrder":["root"],":items":{"root":{":itemsOrder":["text"],":items":{"text":{"id":"text-41bcd02e64","text":"

This is a test

","richText":true,":type":"core/wcm/components/text/v2/text","dataLayer":{"text-41bcd02e64":{"@type":"core/wcm/components/text/v2/text","repo:modifyDate":"2016-01-18T15:59:43Z","xdm:text":"

This is a test

"}}}},":type":"wcm/foundation/components/responsivegrid"}},":type":"core/wcm/components/page/v2/page","dataLayer":{"page-bb590de134":{"@type":"core/wcm/components/page/v2/page","repo:modifyDate":"2016-01-20T09:33:36Z","dc:title":"Templated Page","dc:description":"Description","xdm:template":"/conf/coretest/settings/wcm/templates/product-page","xdm:language":"en-GB","xdm:tags":["three","one","two"],"repo:path":"/core/content/page/templated-page.html"}}}> \
but was: <{"id":"page-bb590de134","lastModifiedDate":1453282416000,"brandSlug":"Brand Slug","description":"Description","title":"Templated Page","designPath":"/etc/designs/mysite","templateName":"product-page","htmlPageItems":[{"attributes":{"href":"/_theme/theme.css","rel":"preload","as":"style"},"element":"link","location":"header"},{"attributes":{"async":true,"defer":false,"src":"/_theme/theme.js"},"element":"script","location":"footer"},{"attributes":{"charset":"UTF-8"},"element":"meta","location":"header"}],"appResourcesPath":"/core/etc.clientlibs/wcm/core/page/clientlibs/favicon/resources","cssClassNames":"class1 class2","language":"en-GB",":items":{"root":{":itemsOrder":["text"],":items":{"text":{"id":"text-41bcd02e64","text":"

This is a test

","richText":true,":type":"core/wcm/components/text/v2/text","dataLayer":{"text-41bcd02e64":{"repo:modifyDate":"2016-01-18T15:59:43Z","@type":"core/wcm/components/text/v2/text","xdm:text":"

This is a test

"}}}},":type":"wcm/foundation/components/responsivegrid"}},":itemsOrder":["root"],":type":"core/wcm/components/page/v2/page","dataLayer":{"page-bb590de134":{"repo:modifyDate":"2016-01-20T09:33:36Z","dc:title":"Templated Page","@type":"core/wcm/components/page/v2/page","dc:description":"Description","xdm:language":"en-GB","xdm:template":"/conf/coretest/settings/wcm/templates/product-page","xdm:tags":["one","three","two"],"repo:path":"/core/content/page/templated-page.html"}}}>
[INFO]
[ERROR] Tests run: 1, Failures: 1, Errors: 0, Skipped: 0
[INFO]
```
- Filtering out the tags from the above log
```
[INFO] Results:
[INFO]
[ERROR] Failures:
[ERROR] PageImplTest.testPage:109->testPage:142
expected: ..."xdm:tags":["three","one","two"]...
but was: ..."xdm:tags":["one","three","two"]...
[INFO]
[ERROR] Tests run: 1, Failures: 1, Errors: 0, Skipped: 0
[INFO]
```

**Expected behavior/code**
- The ordering of the tags should have been maintain in a specific order for all runs
- The tests should have passed successfully when run through `NonDex`

**Environment**
- AEM version and patch level (e.g. AEM 6.3 SP1 CFP2)
- Core Components version (e.g. 2.0.4)
- JRE version (e.g. `Java(TM) SE Runtime Environment (build 1.8.0_152-b16)`)

**Possible Solution**
* Even though the order of key value pair could change in the exported JSON, the order of an array needs to be maintained. So, changing the test function to compare based on the presence of elements in an array (like a Set) is incorrect.
* **Solution**: Sort the ordering of the tags in PageImpl based on the tag name. This ensures that whatever may be order returned by the MockTagManager, we would maintain the same order while exporting to JSON.
```
+/- com.adobe.cq.wcm.core.components.internal.models.v1.PageImpl.java:134

Tag[] tags = currentPage.getTags();
keywords = new String[tags.length];

Tag[] tags = currentPage.getTags();
+ Arrays.sort(tags, Comparator.comparing(Tag::getName));
keywords = new String[tags.length];
```
- However, this also requires to change the expected order of exported tags from ["three", "one", "two"] to ["one", "three", "two"] because of lexicographic order sorting.
- The ordering of tags needs to changed in the following expected JSON files
- `src/test/resources/page/exporter-templated-page.json`
- `src/test/resources/page/v3/exporter-templated-page.json`
- `src/test/resources/page/v3/exporter-templated-page.json`
```
"xdm:tags": [
- "three",
- "one",
"two"
]

"xdm:tags": [
+ "one",
+ "three",
"two"
]
```

**Additional context / Screenshots**
- Check the issue linked at the beginning for additional info if needed.

Contributor guide

Open the contributing guide

Research direction

Start with bundles/core/src/main/java/com/adobe/cq/wcm/core/components/internal/models/v1/PageImpl.java and the listed PageImplTest.testPage variants, then reproduce one failure with the NonDex Maven command. Check the referenced exporter-templated-page.json fixtures; done means tag arrays have deterministic ordering and the affected tests pass under NonDex.

Written by the indexing model from the issue text.

Assessment

Tech stack
java
Domain
backend, testing
Issue type
Bug
Difficulty
2/5
Estimated time
1-3 hours
Activity status
Stale
Clarity
Clearly specified
Newbie friendliness
45/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.