Skip to content

Refactor functional tests to use JSON comparison (part 1)#437

Merged
emdobrin merged 12 commits intoadobe:devfrom
timkimadobe:refactor-functional-tests-aeptestutils2
Jan 27, 2024
Merged

Refactor functional tests to use JSON comparison (part 1)#437
emdobrin merged 12 commits intoadobe:devfrom
timkimadobe:refactor-functional-tests-aeptestutils2

Conversation

@timkimadobe
Copy link
Copy Markdown
Contributor

@timkimadobe timkimadobe commented Jan 9, 2024

Description

This PR updates the first portion of Edge functional tests to use the JSON comparison system. Broadly, it replaces dictionary flattening with JSON comparison. Specific changes:

  1. If flattened dictionary count was applied, then strict collection count checks are applied throughout the JSON validation when using the extensible comparison methods assertExactMatch/assertTypeMatch - that is, the refactor should preserve the same level of strictness for the payload being tested
  2. Everywhere FileManager.default.clearCache() is used, FileManager.default.removeAdobeCacheDirectory() was also added (given Core's new persistence handling)

Questions for reviewers

testSendEvent_receivesResponseEventHandle_sendsResponseEvent_pairedWithTheRequestEventId

  1. Can let requestId = resultNetworkRequests[0].url.queryParam("requestId") ever be nil? Should that be strictly validated? Currently the refactor sets the fallback value to "'

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.

@codecov
Copy link
Copy Markdown

codecov bot commented Jan 9, 2024

Codecov Report

Merging #437 (6fe2426) into dev (6a3563a) will not change coverage.
The diff coverage is n/a.

Additional details and impacted files
@@           Coverage Diff           @@
##              dev     #437   +/-   ##
=======================================
  Coverage   96.47%   96.47%           
=======================================
  Files          27       27           
  Lines        1730     1730           
=======================================
  Hits         1669     1669           
  Misses         61       61           

Comment on lines +284 to +296
assertExactMatch(
expected: expectedJSON,
actual: resultNetworkRequests[0],
pathOptions:
ValueTypeMatch(paths:
"events[0].xdm._id",
"events[0].xdm.timestamp",
"meta.konductorConfig.streaming.lineFeed",
"meta.konductorConfig.streaming.recordSeparator",
"xdm.identityMap.ECID[0].authenticatedState",
"xdm.identityMap.ECID[0].id",
"xdm.identityMap.ECID[0].primary"),
CollectionEqualCount(scope: .subtree))
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When you have multiple paths you are excluded from an exact/type match, is it worth exploring having something like:

assertTypeMatch(expected: expectedCommonKeys, actual: ...)
assertExactMatch(expected: expectedSdkConfigKeys, actual: ...)
assertCount(expected: expectedCommonKeys.count() + expectedSdkConfigKeys.count(), actual: ...)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can create a followup task on the iOS test utils side to explore this option of having separate assertions for each comparison option further!

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok, let's do that if you have more examples like this. if it is just this file, current APIs should suffice

Copy link
Copy Markdown
Contributor Author

@timkimadobe timkimadobe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the review @emdobrin! Updated based on feedback, please let me know what you think of the latest changes

Comment on lines +284 to +296
assertExactMatch(
expected: expectedJSON,
actual: resultNetworkRequests[0],
pathOptions:
ValueTypeMatch(paths:
"events[0].xdm._id",
"events[0].xdm.timestamp",
"meta.konductorConfig.streaming.lineFeed",
"meta.konductorConfig.streaming.recordSeparator",
"xdm.identityMap.ECID[0].authenticatedState",
"xdm.identityMap.ECID[0].id",
"xdm.identityMap.ECID[0].primary"),
CollectionEqualCount(scope: .subtree))
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can create a followup task on the iOS test utils side to explore this option of having separate assertions for each comparison option further!

@timkimadobe timkimadobe requested a review from emdobrin January 27, 2024 01:12
@emdobrin emdobrin merged commit 4b50f2c into adobe:dev Jan 27, 2024
@timkimadobe timkimadobe deleted the refactor-functional-tests-aeptestutils2 branch January 27, 2024 03:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Refactor functional and integration tests to use new test utils

2 participants