Skip to content

Conversation

@mahmoud-elmorabea
Copy link
Contributor

@mahmoud-elmorabea mahmoud-elmorabea commented Mar 21, 2025

Motivation

The strategy we are using to unit test our plugin only using snapshots will not work once we try to run these tests for multiple Expo versions. Different Expo versions generate slightly different native projects which can break snapshot tests.

Solution

I've used a different technique for some of the tests to allow them the flexibility to run on multiple Expo projects.

Gradle files

For Gradle files, we are using gradle-to-js to turn Gradle files into a JSON that we can then assert.

Android manifest

The manifest is already XML that we can parse into structured data and assert items in it

iOS AppDelegate

This is probably the most complicated file we have since it has a lot of code and we inject code within that. I have switched this file to use inline snapshots instead where the snapshot data lives in the test file itself. This allows us to inject different app names in the snapshot.

Pod files

Pod files are not structured and can't really be parsed, so I used the start/end markets we inject to find the lines to assert.

Alternatives I've considered

  • Duplicating tests for each test app: We could have duplicated tests for each test app with expo version we test, but this would have been too much to maintain
  • Regex matching in tests: This gives us the most granular asserting logic but the tests themselves become complex and error prone.

@mahmoud-elmorabea mahmoud-elmorabea self-assigned this Mar 21, 2025
@mahmoud-elmorabea mahmoud-elmorabea marked this pull request as ready for review March 21, 2025 13:43
@mahmoud-elmorabea mahmoud-elmorabea requested a review from a team as a code owner March 21, 2025 13:43
@mahmoud-elmorabea mahmoud-elmorabea force-pushed the unit-test-multi-expo-versions branch from b85fc17 to 7c848d6 Compare March 21, 2025 13:48
@Shahroz16 Shahroz16 requested a review from Copilot March 24, 2025 21:38
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR updates the unit tests to support multiple Expo versions by replacing hard-coded paths with dynamic test utilities and updating snapshot tests to use inline snapshots or fine-grained assertions. Key changes include:

  • Replacing static "test-app" paths with dynamic testAppPath() (and in some cases testAppName()).
  • Replacing full snapshot comparisons with inline snapshots or explicit line assertions for iOS and Android configuration files.
  • Updating Gradle and manifest tests to parse and validate structured data rather than relying on snapshots.

Reviewed Changes

Copilot reviewed 22 out of 27 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
tests/ios/apn/PodFile.test.js Updated to use testAppPath() and more explicit pod block assertions.
tests/ios/fcm/AppDelegate-impl.test.js Switched from snapshot test to inline snapshot with dynamic app name import.
tests/ios/apn/AppDelegate-impl.test.js Similar inline snapshot update with dynamic path resolution.
tests/ios/fcm/PodFile.test.js Updated pod assertions for FCM and dependency naming.
tests/android/app-manifest.test.js Refactored manifest test to use XML parsing for service validation.
tests/android/app-gradle-build.test.js Changed to use gradle-to-js for plugin verification.
tests/android/main-gradle-build.test.js Added dependency checks via parsed Gradle JSON and verified repository URL.
tests/ios/common/AppDelegate-header.test.js, GoogleService-InfoCopied.test.js, NotificationService*, PushService-swift.test.js Updated path resolution using testAppPath() (and testAppName()) to remove hard-coded paths.
Files not reviewed (5)
  • tests/android/snapshots/app-gradle-build.test.js.snap: Language not supported
  • tests/android/snapshots/app-manifest.test.js.snap: Language not supported
  • tests/android/snapshots/main-gradle-build.test.js.snap: Language not supported
  • tests/ios/apn/snapshots/AppDelegate-impl.test.js.snap: Language not supported
  • tests/ios/apn/snapshots/PodFile.test.js.snap: Language not supported


test("Plugin injects expted dependencies in the main Gradle build file", async () => {
const content = await fs.readFile(mainBuildGradlePath, "utf8");
test('Plugin injects expted dependencies in the main Gradle build file', async () => {
Copy link

Copilot AI Mar 24, 2025

Choose a reason for hiding this comment

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

Typo in the test description: 'expted' should be corrected to 'expected'.

Suggested change
test('Plugin injects expted dependencies in the main Gradle build file', async () => {
test('Plugin injects expected dependencies in the main Gradle build file', async () => {

Copilot uses AI. Check for mistakes.
Copy link
Contributor

@Shahroz16 Shahroz16 left a comment

Choose a reason for hiding this comment

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

Very happy with the direction we are taking and instead of relying on snapshots we are breaking down problem and solving them as they seem fit. I think we can further enhance it so I have created a follow up PR which shouldn't block it but would serve as something built on top of it.

package.json Outdated
Comment on lines 129 to 130
"fs-extra": "^11.2.0",
"gradle-to-js": "^2.0.1"
Copy link
Contributor

Choose a reason for hiding this comment

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

Will this add dependencies to our plugin that will be shipped to customers? Can't we use devDependencies or something similar to avoid this?

Copy link
Contributor

@mrehan27 mrehan27 left a comment

Choose a reason for hiding this comment

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

Looks good 👍🏻

@mahmoud-elmorabea mahmoud-elmorabea merged commit d6e6a68 into beta Mar 25, 2025
8 checks passed
@mahmoud-elmorabea mahmoud-elmorabea deleted the unit-test-multi-expo-versions branch March 25, 2025 09:10
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.

4 participants