Skip to content

Integration test cases#101

Merged
timkimadobe merged 29 commits intoadobe:feature/integration-testsfrom
timkimadobe:integration-test-cases3
Sep 18, 2023
Merged

Integration test cases#101
timkimadobe merged 29 commits intoadobe:feature/integration-testsfrom
timkimadobe:integration-test-cases3

Conversation

@timkimadobe
Copy link
Copy Markdown
Contributor

Description

This PR implements the integration test cases, achieving parity with the iOS integration tests: adobe/aepsdk-edge-ios#346

The environment file ID used is the same as on the iOS side.

Key differences (that should probably be ported to the iOS version):

  1. the use of private fun resetTestExpectations() within the test class to make managing multiple helper resets easier
  2. calling realNetworkService.assertAllNetworkRequestExpectations() before getResponseFor, as the latter method is synchronous

Notable changes:

  1. testSendEvent_withInvalidDatastreamID_receivesExpectedError consistently failing because of a race condition between configuration pending shared state and edge sendEvent event processing
    • Addressed using a wait on all expected configuration response content events before performing sendEvent
  2. use of Content-Length header value in testSendEvent_withInvalidLocationHint_receivesExpectedError to double validate that server intent is to send 0 bytes in the response body
    • Note to reviewers: would like to get opinions on if this is required or if the null check on inputStream is enough?

TODO:

  1. properly set the makefile integration test rule args from the GitHub Action workflow script

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.

kevinlind and others added 23 commits August 16, 2023 18:09
* Fix deprecated use of

* Fix Groovy warnings in build files
* Send 'complete' event after all response handles are recieved when requested

* Update event source variable names to match source name 'CONTENT_COMPLETE'

* Reword method description for sendCompletionRequested

* Add more tests to bump coverage numbers

* Add Content Complete event to event-reference documentation

* Update event-reference with sendCompletion property and response content definition.

* Capitalize 'Defined' in table for completion event description
Add content complete event to toc, update response content description
Merge to Staging for v2.3.0 release
Using 'asNode()' gives warning about implicit null value, however passing 'asNode(null)' fails to build publish step.
Revert 'asNode(null)' to 'asNode()'.
Merge to Staging for 2.3.0 Gradle fix for publish
…be#89)

* Set Core dependency to 2.4.0

* Replace hardcoded event source with EventSource.CONTENT_COMPLETE

* Fix failing deep copy test after Core changes to ignore invalid objects.
Merge to Staging for 2.4.0 release, updates Core dependency to 2.4.0
Merge to Main for 2.3.0 release
… testable network request

First integration test case
@emdobrin
Copy link
Copy Markdown
Contributor

@timkimadobe unit tests are failing at testDeepCopy_whenInvalidMapWithCustomObjects_returnsOnlyValidObjects which was fixed in dev. Can you synchronize the changes from dev into your feature branch?

@emdobrin
Copy link
Copy Markdown
Contributor

to your question above, what you currently have for checking content length and bites looks ok to me

Copy link
Copy Markdown
Contributor

@emdobrin emdobrin left a comment

Choose a reason for hiding this comment

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

Looks great and thanks for preserving the consistency with the ios suite 💯

Edge.sendEvent(experienceEvent) {}

realNetworkService.assertAllNetworkRequestExpectations()
val matchingResponse = realNetworkService.getResponseFor(interactNetworkRequest)
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.

should this be getResponsesFor and return an array with all matches for this url? then we can also assert the number is greater than the expectation. This comment applies for all other tests the get the responses for asserts.

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.

Yes I think that sounds great! Refactored to make the responses a list, and updated test expectations accordingly

# Conflicts:
#	code/app/build.gradle
#	code/edge/build.gradle
#	code/edge/src/androidTest/java/com/adobe/marketing/mobile/EdgeFunctionalTests.java
@timkimadobe
Copy link
Copy Markdown
Contributor Author

timkimadobe commented Sep 15, 2023

@timkimadobe unit tests are failing at testDeepCopy_whenInvalidMapWithCustomObjects_returnsOnlyValidObjects which was fixed in dev. Can you synchronize the changes from dev into your feature branch?

Merged dev-v2.3.1 into this branch to bring in latest updates!
Edit: looks like the merge blew up the diff 🙈

@timkimadobe timkimadobe linked an issue Sep 15, 2023 that may be closed by this pull request
@codecov
Copy link
Copy Markdown

codecov bot commented Sep 15, 2023

Codecov Report

Merging #101 (0cb0c97) into feature/integration-tests (d390fd1) will decrease coverage by 0.04%.
Report is 5 commits behind head on feature/integration-tests.
The diff coverage is 100.00%.

@@                       Coverage Diff                       @@
##             feature/integration-tests     #101      +/-   ##
===============================================================
- Coverage                        83.63%   83.59%   -0.04%     
- Complexity                         381      385       +4     
===============================================================
  Files                               29       29              
  Lines                             1582     1578       -4     
  Branches                           225      223       -2     
===============================================================
- Hits                              1323     1319       -4     
- Misses                             162      165       +3     
+ Partials                            97       94       -3     
Flag Coverage Δ
functional-tests 66.48% <92.31%> (+0.29%) ⬆️
unit-tests 79.53% <100.00%> (-0.11%) ⬇️

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

Files Changed Coverage Δ
...java/com/adobe/marketing/mobile/EdgeConstants.java 100.00% <ø> (ø)
...a/com/adobe/marketing/mobile/EdgeHitProcessor.java 87.22% <100.00%> (-0.37%) ⬇️
...adobe/marketing/mobile/NetworkResponseHandler.java 84.62% <100.00%> (+1.71%) ⬆️

... and 1 file with indirect coverage changes

responseConnection: HttpConnecting
) {
networkResponses[request] = responseConnection
if (networkResponses[request] != null) {
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.

  • nit: equivalent to this API in iOS is recordSentNetworkRequest
  • I think all equality checks on network requests in this file will fail without using a custom equality check similar to iOS, or do we already have a custom isEqual in Android?

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 believe this is the same as setResponseFor
This is the one that records the network response for a given request
I've renamed the method on Android to match accordingly!

There is a custom equals implementation already in place for the TestableNetworkRequest class which also provides a hash implementation for dictionaries to use both of these custom implementations to manage keys

I think there could be benefit to bringing this style implementation to iOS, since we can have the benefit of a new TestableNetworkRequest: NetworkRequest without overwriting the default equals and hashable implementation of NetworkRequest, which we tried to avoid by using a custom isCustomEquals method which I think is a bit clumsier

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.

Yup and in addition iOS would also need to handle a list of responses for a particular request

@timkimadobe timkimadobe merged commit da2ebc3 into adobe:feature/integration-tests Sep 18, 2023
@timkimadobe timkimadobe mentioned this pull request Sep 18, 2023
10 tasks
@timkimadobe timkimadobe deleted the integration-test-cases3 branch September 22, 2023 00:24
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.

Implement test cases based on test plan

3 participants