Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Transfer AEPTestUtils to Core #723

Merged
merged 7 commits into from
Nov 8, 2024
Merged

Transfer AEPTestUtils to Core #723

merged 7 commits into from
Nov 8, 2024

Conversation

timkimadobe
Copy link
Contributor

@timkimadobe timkimadobe commented Nov 8, 2024 •
edited
Loading

Description

This PR transfers over AEPTestUtils to the Core repo as a separate module. At a high level the updates include:

  1. Creation of new testutils module with current AEPTestUtils implementation from Android AEPTestUtils: https://github.com/timkimadobe/aepsdk-testutils-android
  2. Gradle updates to:
    1. Include testutils as a project module
    2. Add AEPTestUtils to Core test and androidTest dependencies
  3. Gradle config for AEPTestUtils
    1. Include Core as a local project dependency
    2. In publishing, add all other non-Core dependencies as Maven dependencies
  4. Makefile updates
    1. Add AEPTestUtil related make rules (excluding checkstyle)
  5. API dump for AEPTestUtils - file created using gradle apiDump

All other new files are transfers from AEPTestUtils existing implementation (unchanged).

Tested this configuration by adding the following code snippets using a class from AEPTestUtils to Core tests:
Unit test: MobileCoreTests.kt - testDispatchEventSimple()
Functional test: DataMarshallerTests.kt - marshalDeepLinkData()

val latch = ADBCountDownLatch(1)
latch.countDown()
assertEquals(0, latch.initialCount - latch.currentCount)

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.

@timkimadobe timkimadobe requested a review from praveek November 8, 2024 02:26
@timkimadobe timkimadobe linked an issue Nov 8, 2024 that may be closed by this pull request
Migrate Android AEPTestUtils to Core Android #721
Open
@codecov Codecov
Copy link

codecov bot commented Nov 8, 2024 •
edited
Loading

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 81.65%. Comparing base (89b4cac) to head (66effd3).
Report is 1 commits behind head on dev.

Additional details and impacted files 
@@             Coverage Diff              @@
##                dev     #723      +/-   ##
============================================
+ Coverage     81.61%   81.65%   +0.03%     
- Complexity     2151     2154       +3     
============================================
  Files           192      192              
  Lines          8979     8979              
  Branches       1124     1124              
============================================
+ Hits           7328     7331       +3     
  Misses         1093     1093              
+ Partials        558      555       -3
Flag Coverage Δ
functional-tests 27.82% <ø> (-0.01%) ⬇️
unit-tests 69.25% <ø> (+0.03%) ⬆️

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

see 2 files with indirect coverage changes

praveek
praveek reviewed Nov 8, 2024
View reviewed changes
.circleci/config.yml
@@ -9,7 +9,7 @@ jobs:
executor:
name: android/android-docker
resource-class: large
tag: 2024.01.1
tag: 2024.01.1-node
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this necessary?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This one is only necessary to allow test utils: enableSpotlessPrettierForJava = true
Without it, the spotless java check fails because the CI runner can't find a valid npm instance

@praveek
Copy link
Contributor

praveek commented Nov 8, 2024 •
edited
Loading

Can you also update the jitpack.yml file to include publishing the test utilities?

@praveek praveek changed the base branch from dev-testutils to dev November 8, 2024 20:27
@praveek
Copy link
Contributor

praveek commented Nov 8, 2024

The changes look good. Let’s reuse the dev branch instead of maintaining a separate dev-testutils branch for Android.
Add a GitHub issue to track updating the version.yml GitHub Action to include the test utils version. This update can be made alongside other CI changes.

@timkimadobe
Copy link
Contributor Author

Thanks for the review @praveek! Updated based on feedback, and created the follow up task to update versions.yml with AEPTestUtils support: #725

@timkimadobe timkimadobe requested a review from praveek November 8, 2024 21:32
@praveek praveek merged commit 07af668 into adobe:dev Nov 8, 2024
8 checks passed
@timkimadobe timkimadobe deleted the dev-testutils branch November 8, 2024 22:53
@timkimadobe timkimadobe restored the dev-testutils branch November 8, 2024 22:53
@timkimadobe timkimadobe deleted the dev-testutils branch November 15, 2024 22:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@praveek praveek praveek approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Migrate Android AEPTestUtils to Core Android
2 participants
@timkimadobe @praveek