Skip to content

[AMSDK-10056] Adds e2e tests for AEPExperiencePlatform extension#48

Merged
emdobrin merged 17 commits intoadobe:devfrom
emdobrin:AMSDK-10056
Jul 20, 2020
Merged

[AMSDK-10056] Adds e2e tests for AEPExperiencePlatform extension#48
emdobrin merged 17 commits intoadobe:devfrom
emdobrin:AMSDK-10056

Conversation

@emdobrin
Copy link
Copy Markdown
Contributor

Description

Adds e2e tests for AEPExperiencePlatform extension covering the main flows with:

  • network request format assertions
  • request/response event assertions
  • client-side store assertions

Fixed a bug where the request event was dispatched when xdm data was empty [:]

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)
  • New e2e tests, increased coverage

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.

@emdobrin emdobrin requested a review from kevinlind July 13, 2020 19:29
super.setUp()
FunctionalTestUtils.resetUserDefaults()
continueAfterFailure = false
if AEPExperiencePlatformFunctionalTests.firstRun {
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.

I'm wondering if this would be cleaner if FunctionalTestBase contained a method firstRun() which is called from FunctionalTestBase.setUp()? Each test class can then override func firstRun() with this first run logic. I think it would be easier for developers to add this logic to a firstRun() method instead of needed to implement this if block.

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.

one solution would be to move these steps on the class setUp method, but it requires the framework APIs to be static which I wanted to avoid, it feels more convenient to have them on the instance and call them directly. Not sure how I feel about having another method firstRun on top of class setUp 🤔

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.

I still feel it would be cleaner to separate the "first run" logic from the "run for every test" logic. However, we can think about it more with we need to implement more functional tests if you'd like.
Another issue: is it possible to use FunctionTestBase.firstRun instead of defining your own variable? Maybe create a public readonly property isFirstRun ?

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.

Changed based on your last comment. We might be able to define a protocol and add methods we want child classes to implement, but I would not include those changes as part of this pr.


guard let eventData = experiencePlatformEvent.asDictionary() else {
ACPCore.log(ACPMobileLogLevel.debug, tag: LOG_TAG, message:"Failed to dispatch the event because the event data is nil.")
guard let xdmData = experiencePlatformEvent.xdm, !xdmData.isEmpty, let eventData = experiencePlatformEvent.asDictionary() else {
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.

👍


extension Int {
init(_ val:Bool) {
self = val ? 1 : 0
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.

What's the use of this? Are you seeing that Ints 1 & 0 are being converted to booleans after being serialized through the Event Hub?

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.

just the events come out of order each time, and since I only had two events I used true/false to check the order 😆 is it too confusing?

@codecov
Copy link
Copy Markdown

codecov bot commented Jul 16, 2020

Codecov Report

Merging #48 into dev will increase coverage by 6.57%.
The diff coverage is 87.50%.

@@            Coverage Diff             @@
##              dev      #48      +/-   ##
==========================================
+ Coverage   71.34%   77.90%   +6.57%     
==========================================
  Files          29       29              
  Lines         949      973      +24     
==========================================
+ Hits          677      758      +81     
+ Misses        272      215      -57     

@emdobrin emdobrin requested a review from kevinlind July 17, 2020 18:43
Copy link
Copy Markdown
Contributor

@kevinlind kevinlind left a comment

Choose a reason for hiding this comment

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

Added another minor comment, but overall this looks great.

@emdobrin emdobrin merged commit a6a5662 into adobe:dev Jul 20, 2020
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.

2 participants