search

adobe / aem-core-wcm-components

Standardized components to build websites with AEM.
https://docs.adobe.com/content/help/en/experience-manager-core-components/using/introduction.html
Apache License 2.0
726 stars 735 forks source link

[Client Library] Non-deterministic behavior of `HashSet` might fail the test `ClientLibrariesImplTest.testGetCategoriesWithInjectedResourceTypesAndInheritance` #2606

Closed SaaiVenkat closed 6 months ago

SaaiVenkat commented 8 months ago

Bug Report

Current Behavior

Steps to Reproduce

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=com.adobe.cq.wcm.core.components.internal.models.v1.ClientLibrariesImplTest#testGetCategoriesWithInjectedResourceTypesAndInheritance

Run the unit test using NonDex

mvn -pl bundles/core edu.illinois:nondex-maven-plugin:2.1.1:nondex -Dtest=com.adobe.cq.wcm.core.components.internal.models.v1.ClientLibrariesImplTest#testGetCategoriesWithInjectedResourceTypesAndInheritance

- Reproducing the above steps results in the following error

[INFO] Results: [INFO] [ERROR] Failures: [ERROR] ClientLibrariesImplTest.testGetCategoriesWithInjectedResourceTypesAndInheritance:464 \ expected: <> \ but was: <> [INFO] [ERROR] Tests run: 1, Failures: 1, Errors: 0, Skipped: 0



**Expected behavior/code**
- The order of includes should have been 
  1) `jsInclude of ACCORDIAN` category 
  2) `jsInclude of CAROUSEL` category
  3) `cssInclude of ACCORDIAN` category 
  4) `cssInclude of CAROUSEL` category 
- The test should have passed successfully without any errors with `NonDex`.

**Actual behaviour**
- The order of elements could be either of the following depending on the implementation of `HashSet`
- 1st Possible order
  1) `jsInclude of ACCORDIAN` category 
  2) `jsInclude of CAROUSEL` category
  3) `cssInclude of ACCORDIAN` category 
  4) `cssInclude of CAROUSEL` category 
- 2nd Possible order
  1) `jsInclude of CAROUSEL` category
  2) `jsInclude of ACCORDIAN` category 
  3) `cssInclude of CAROUSEL` category 
  4) `cssInclude of ACCORDIAN` category 

**Environment**
- Core Components version: `2.23.5-SNAPSHOT`
- Java version: `Java(TM) SE Runtime Environment (build 1.8.0_381-b09)`

**Possible Solution**
<!--- If you have suggestions for the bug fix. Please also consider creating a Pull Request 🤘. -->
There are two possible solutions based on the requirements
- **Possible Solution 1**
  - *Requirement*: Order of the categories need to be maintained
  - *Solution*: Changing the `HashSet` to `LinkedHashSet` at https://github.com/adobe/aem-core-wcm-components/blob/3524e33424bf6831faca014284c5bad803d3744d/bundles/core/src/main/java/com/adobe/cq/wcm/core/components/internal/models/v1/ClientLibrariesImpl.java#L300
  - When `categoriesArray` is built from `categoriesSet`, this ensures that the order or categories will be maintained.
- **Possible Solution 2**
  - *Requirement*: Order of the categories is not important
  - *Solution*: 
    - The current implementation of the mentioned test expects the `ClientLibraries.getJsAndCssIncludes()` to be in the order `ACCORDION` includes followed by `CAROUSEL` includes at https://github.com/adobe/aem-core-wcm-components/blob/3524e33424bf6831faca014284c5bad803d3744d/bundles/core/src/test/java/com/adobe/cq/wcm/core/components/internal/models/v1/ClientLibrariesImplTest.java#L458-L464
    - This assertion could be modified to include both the orders, i.e `ACCORDION` includes followed by `CAROUSEL` includes as well as `CAROUSEL` includes followed by `ACCORDION` includes.
- **If the proposed solution looks good, I can create a PR**

**Additional context / Screenshots**

- 1st order given by `HashSet`
 <img width="728" alt="Screenshot 2023-10-30 at 12 42 14 PM" src="https://github.com/adobe/aem-core-wcm-components/assets/46614118/0e13d8ef-ff24-44c2-89db-622160081fb8">

- `categoriesArray` set from `getCategoriesFromComponents()`
<img width="739" alt="Screenshot 2023-10-30 at 12 42 50 PM" src="https://github.com/adobe/aem-core-wcm-components/assets/46614118/c35d0277-a77b-49c3-bf7d-452d0de80357">

- `getJsAndCssIncludes()` generated for the first order
<img width="597" alt="Screenshot 2023-10-30 at 12 43 53 PM" src="https://github.com/adobe/aem-core-wcm-components/assets/46614118/5aa73f3a-98c0-4fb6-af0a-31fa9d9621e2">

- 2nd order given by `HashSet`
<img width="706" alt="Screenshot 2023-10-30 at 12 46 02 PM" src="https://github.com/adobe/aem-core-wcm-components/assets/46614118/9990c4d4-9ae5-4031-9d42-586c6f106ba3">

- `categoriesArray` set from `getCategoriesFromComponents()`
<img width="673" alt="Screenshot 2023-10-30 at 12 46 20 PM" src="https://github.com/adobe/aem-core-wcm-components/assets/46614118/fdbe903e-2a63-4d6e-8a3c-3b568c020e6c">

- `getJsAndCssIncludes()` generated for the second order
<img width="600" alt="Screenshot 2023-10-30 at 12 46 47 PM" src="https://github.com/adobe/aem-core-wcm-components/assets/46614118/5d2e86ba-5b51-4183-88c4-5b5f964a9fca">