Integration test JSON comparison system by timkimadobe · Pull Request #79 · adobe/aepsdk-edge-android

timkimadobe · 2023-08-08T20:25:04Z

Description

This PR implements the JSON comparison system analogous to the version built in Edge iOS (adobe/aepsdk-edge-ios#332). Instead of AnyCodable, this version uses JSONObject and JSONArray from the org.json package.

Related Issue

Motivation and Context

How Has This Been Tested?

Screenshots (if appropriate):

Types of changes

Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

I have signed the Adobe Open Source CLA.
My code follows the code style of this project.
My change requires a change to the documentation.
I have updated the documentation accordingly.
I have read the CONTRIBUTING document.
I have added tests to cover my changes.
All new and existing tests passed.

Slight logical errors and mutability

…blic methods Add initial unit test cases for JSON comparison assertion methods

codecov · 2023-08-08T20:38:38Z

Codecov Report

Merging #79 (3af325e) into feature/integration-tests (6483999) will not change coverage.
The diff coverage is n/a.

@@                     Coverage Diff                      @@
##             feature/integration-tests      #79   +/-   ##
============================================================
  Coverage                        83.63%   83.63%           
  Complexity                         380      380           
============================================================
  Files                               29       29           
  Lines                             1582     1582           
  Branches                           225      225           
============================================================
  Hits                              1323     1323           
  Misses                             162      162           
  Partials                            97       97

Flag	Coverage Δ
functional-tests	`66.18% <ø> (ø)`
unit-tests	`79.65% <ø> (ø)`

Flags with carried forward coverage won't be shown. Click here to find out more.

code/test-utils/build.gradle

code/upstream-integration-tests/build.gradle

code/test-utils/src/main/java/com/adobe/marketing/tester/util/JSONObjectAsserts.kt

...tests/src/androidTest/java/com/adobe/marketing/mobile/integration/ExampleInstrumentedTest.kt

code/test-utils/src/test/java/com/adobe/marketing/tester/util/JSONObjectAssertsTests.kt

code/test-utils/src/main/java/com/adobe/marketing/tester/util/JSONObjectAsserts.kt

kevinlind · 2023-08-10T18:02:02Z

code/test-utils/build.gradle

 dependencies {
    implementation project(':edge')
    implementation "com.adobe.marketing.mobile:core:${rootProject.mavenCoreVersion}"
+    implementation "androidx.test.ext:junit:${rootProject.ext.junitVersion}"


Are the Junit libraries needed under implementation or only testImplementation?

There are some direct assertions in the comparison code itself which require it in the implementation

code/upstream-integration-tests/build.gradle

Update comparison logic

Remove extra newlines in assertion logging

Refactor alternate path array access handling and add corresponding test cases

timkimadobe

Thanks @kevinlind and @emdobrin for the review! Updated based on feedback

timkimadobe · 2023-08-18T00:27:01Z

code/test-utils/build.gradle

 dependencies {
    implementation project(':edge')
    implementation "com.adobe.marketing.mobile:core:${rootProject.mavenCoreVersion}"
+    implementation "androidx.test.ext:junit:${rootProject.ext.junitVersion}"


There are some direct assertions in the comparison code itself which require it in the implementation

code/test-utils/build.gradle

code/test-utils/src/main/java/com/adobe/marketing/tester/util/JSONObjectAsserts.kt

code/test-utils/src/test/java/com/adobe/marketing/tester/util/JSONObjectAssertsTests.kt

...tests/src/androidTest/java/com/adobe/marketing/mobile/integration/ExampleInstrumentedTest.kt

code/upstream-integration-tests/build.gradle

code/test-utils/src/main/java/com/adobe/marketing/tester/util/JSONAsserts.kt

addb · 2023-08-21T22:22:38Z

code/test-utils/src/main/java/com/adobe/marketing/tester/util/JSONAsserts.kt

+                    expected = expected.opt(key),
+                    actual = actual.opt(key),
+                    keyPath = keyPath,
+                    pathTree = null, // is String means path terminates here


typo: "null String means path.." ?

Consolidated the logic here, hopefully it is more clear!

Replace all nil with null

timkimadobe

Thanks for the review @addb! Updated based on feedback
Also refactored some logic to simplify code paths and updated a lot of the method documentation to improve:

Consistency of param descriptions across methods
Consistency of code/class styling
Descriptions of methods, especially the public ones

code/test-utils/src/main/java/com/adobe/marketing/tester/util/JSONAsserts.kt

timkimadobe · 2023-08-23T02:30:13Z

code/test-utils/src/main/java/com/adobe/marketing/tester/util/JSONAsserts.kt

+                    expected = expected.opt(key),
+                    actual = actual.opt(key),
+                    keyPath = keyPath,
+                    pathTree = null, // is String means path terminates here


Consolidated the logic here, hopefully it is more clear!

…unit 4.13.2

code/test-utils/src/main/java/com/adobe/marketing/tester/util/JSONAsserts.kt

emdobrin · 2023-08-31T01:10:34Z

code/test-utils/src/test/java/com/adobe/marketing/tester/util/JSONAssertsTests.kt

+    }
+
+    @Test
+    fun `test special key name trailing period`() {


since you are running same asserts for all these special characters I think you can write these as parameterized tests where you predefine the expected/actual values.
this will save some lines of code as well and show all the values tested in one place

That sounds great! Updated to use parameterized tests for test cases where there was a lot of overlap in terms of setup. The current setup uses a larger class to hold the individual parameterized cases since Junit 4 only supports sending parameters to each test case. It also uses a test suite setup so all the individual parameterized test classes can be run at once

wow, after these changes 1k lines were gone! 🎉
I left some comments on the ones that looked more intimidating, please review to see if they make sense to be kept in the expanded form.

code/test-utils/src/test/java/com/adobe/marketing/tester/util/JSONAssertsTests.kt

Update wildcard matching docs

Update to test fail instead of print

Remove redundant test cases from JSONAssertsTests, simplify cases based on already covered logic

timkimadobe

Thank you for the review @emdobrin! Updated based on feedback, hopefully the parameterized tests help improve the readability. Please let me know what you think of the new test organization and logical coverage!

code/test-utils/src/main/java/com/adobe/marketing/tester/util/JSONAsserts.kt

timkimadobe · 2023-09-01T00:12:06Z

code/test-utils/src/test/java/com/adobe/marketing/tester/util/JSONAssertsTests.kt

+    }
+
+    @Test
+    fun `test special key name trailing period`() {


That sounds great! Updated to use parameterized tests for test cases where there was a lot of overlap in terms of setup. The current setup uses a larger class to hold the individual parameterized cases since Junit 4 only supports sending parameters to each test case. It also uses a test suite setup so all the individual parameterized test classes can be run at once

code/test-utils/src/test/java/com/adobe/marketing/tester/util/JSONAssertsTests.kt

Update method docs

code/test-utils/src/test/java/com/adobe/marketing/tester/util/JSONAssertsParameterizedTests.kt

code/test-utils/src/test/java/com/adobe/marketing/tester/util/JSONAssertsTests.kt

emdobrin · 2023-09-02T00:50:18Z

code/test-utils/src/test/java/com/adobe/marketing/tester/util/JSONAssertsTests.kt

+    }
+
+    @Test
+    fun `test special key name trailing period`() {


wow, after these changes 1k lines were gone! 🎉
I left some comments on the ones that looked more intimidating, please review to see if they make sense to be kept in the expanded form.

timkimadobe

Thanks for the review @emdobrin! Updated based on feedback with some outstanding questions, please let me know what you think

code/test-utils/src/test/java/com/adobe/marketing/tester/util/JSONAssertsParameterizedTests.kt

code/test-utils/src/test/java/com/adobe/marketing/tester/util/JSONAssertsTests.kt

code/test-utils/src/test/java/com/adobe/marketing/tester/util/JSONAssertsParameterizedTests.kt

timkimadobe added 7 commits August 4, 2023 11:42

Exact equality assertion methods for JSON comparison

03904a0

Fix merge by removing duplicate code sections

f4fcdc0

Add license header to files

39e0cc1

Add flexible comparison methods

f7c8d2d

Fix implementation

ad1da6f

Slight logical errors and mutability

Update code documentation

5391d32

Refactor asserts from class to object and add static annotation to pu…

3346366

…blic methods Add initial unit test cases for JSON comparison assertion methods

emdobrin reviewed Aug 10, 2023

View reviewed changes

kevinlind reviewed Aug 10, 2023

View reviewed changes

timkimadobe added 5 commits August 17, 2023 17:21

Add more test cases for JSON comparison system

66b2ac2

Update comparison logic

Rename JSON comparison class and usages to be more generic

6136e9a

Remove extra newlines in assertion logging

Update helpers to have more inline documentation

b6daae8

Refactor alternate path array access handling and add corresponding test cases

Cover additional edge cases

df46cce

Clean up upstream-integration-tests gradle and test file

f13e6f8

timkimadobe commented Aug 18, 2023

View reviewed changes

Remove unused dependency

f32c830

timkimadobe mentioned this pull request Aug 18, 2023

Integration test network service helpers #82

Closed

10 tasks

addb reviewed Aug 21, 2023

View reviewed changes

code/test-utils/src/main/java/com/adobe/marketing/tester/util/JSONAsserts.kt Outdated Show resolved Hide resolved

addb reviewed Aug 21, 2023

View reviewed changes

code/test-utils/src/main/java/com/adobe/marketing/tester/util/JSONAsserts.kt Outdated Show resolved Hide resolved

addb reviewed Aug 21, 2023

View reviewed changes

code/test-utils/src/main/java/com/adobe/marketing/tester/util/JSONAsserts.kt Outdated Show resolved Hide resolved

addb reviewed Aug 21, 2023

View reviewed changes

timkimadobe added 2 commits August 22, 2023 17:21

Fix logical condition in test failure output

d87aaea

Replace all nil with null

Clean up implementation and improve documentation

dbbea2c

timkimadobe commented Aug 23, 2023

View reviewed changes

timkimadobe added 2 commits August 22, 2023 19:33

Add more description for wildcard matching behavior

beb769b

Update androidx.text.ext:junit junitVersion to 1.1.5 which provides j…

008db58

…unit 4.13.2

emdobrin reviewed Aug 31, 2023

View reviewed changes

timkimadobe added 2 commits August 31, 2023 11:14

Update alternate path documentation

88d6228

Update wildcard matching docs

Simplify boolean logic using local variables, update usages

7770a4c

Update to test fail instead of print

Add parameterized tests to remove boilerplate

4418a73

Remove redundant test cases from JSONAssertsTests, simplify cases based on already covered logic

timkimadobe commented Sep 1, 2023

View reviewed changes

timkimadobe added 4 commits September 1, 2023 14:24

Add array wildcard to tested alternate path cases

2c0c57f

Remove redundant test cases covered by parameterized tests

6e23132

Update method docs

Update method docs and test case names

fcaf8b0

Method doc updates

00ecf21

emdobrin reviewed Sep 2, 2023

View reviewed changes

timkimadobe added 2 commits September 5, 2023 15:23

Update to improve test case readability for parameterized tests

0c27c93

Add more key value pairs to chained alternate path test case

f8ffec8

timkimadobe commented Sep 5, 2023

View reviewed changes

Update to improve test case readability

e9ebd18

emdobrin reviewed Sep 5, 2023

View reviewed changes

code/test-utils/src/test/java/com/adobe/marketing/tester/util/JSONAssertsParameterizedTests.kt Outdated Show resolved Hide resolved

timkimadobe added 4 commits September 5, 2023 16:38

Expand test cases for clarity

81c406d

Split test cases by JSONObject JSONArray type

de0194f

Add null expected check

09e4f54

Update test debug helper array index comments

3af325e

emdobrin approved these changes Sep 6, 2023

View reviewed changes

timkimadobe merged commit ad87367 into adobe:feature/integration-tests Sep 6, 2023

timkimadobe deleted the integration-test-json-comparison branch September 6, 2023 01:24

timkimadobe linked an issue Sep 8, 2023 that may be closed by this pull request

Add JSON comparison assert helpers #64

Closed

This was referenced Sep 18, 2023

JSON comparison feature parity adobe/aepsdk-edge-ios#400

Closed

JSON comparison parity (utility only) adobe/aepsdk-edge-ios#401

Merged

Feature/integration tests #103

Merged

JSON comparison parity - test cases adobe/aepsdk-edge-ios#402

Merged

Conversation

timkimadobe commented Aug 8, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Related Issue

Motivation and Context

How Has This Been Tested?

Screenshots (if appropriate):

Types of changes

Checklist:

Uh oh!

codecov bot commented Aug 8, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

timkimadobe left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

timkimadobe left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

timkimadobe left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

timkimadobe commented Aug 8, 2023 •

edited

Loading

codecov bot commented Aug 8, 2023 •

edited

Loading