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.

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

Do not attempt to search for manifest in test folders #2075

Conversation

cortinico
Copy link
Member

Summary:

The CLI should not attempt to look into test folders to grab manifests from there.
Specifically this is broken in the react-native repo as we do have those two Manifests:

packages/rn-tester/android/app/src/debug/AndroidManifest.xml
packages/rn-tester/android/app/src/main/AndroidManifest.xml

With this change we prevent the CLI from searching inside src/test/ and src/androidTest.

Test Plan:

I've tested this on main of react-native and yarn android now works well.

Checklist

  • Documentation is up to date to reflect these changes.
  • Follows commit message convention described in CONTRIBUTING.md

cortinico added a commit to cortinico/react-native that referenced this pull request Sep 12, 2023
Summary:
`yarn android` on main is currently broken due to the CLI attempting to search for the manifest inside the src/androidTest folder.
This fixes it by specifying the exact path of the Android Manifest.
The fix to the CLI is also pending to prevent the CLI from searching inside test folders.

Fix for the CLI is here:
- react-native-community/cli#2075

Changelog:
[Internal] [Changed] - Unblock `yarn android` on main

Differential Revision: D49190626

fbshipit-source-id: 26fcdb13147f992451d428a9b8dd886c0cb5bfd5
cortinico added a commit to cortinico/react-native that referenced this pull request Sep 12, 2023
Summary:

`yarn android` on main is currently broken due to the CLI attempting to search for the manifest inside the src/androidTest folder.
This fixes it by specifying the exact path of the Android Manifest.
The fix to the CLI is also pending to prevent the CLI from searching inside test folders.

Fix for the CLI is here:
- react-native-community/cli#2075

Changelog:
[Internal] [Changed] - Unblock `yarn android` on main

Reviewed By: huntie

Differential Revision: D49190626
cortinico added a commit to cortinico/react-native that referenced this pull request Sep 13, 2023
Summary:

`yarn android` on main is currently broken due to the CLI attempting to search for the manifest inside the src/androidTest folder.
This fixes it by specifying the exact path of the Android Manifest.
The fix to the CLI is also pending to prevent the CLI from searching inside test folders.

Fix for the CLI is here:
- react-native-community/cli#2075

Changelog:
[Internal] [Changed] - Unblock `yarn android` on main

Reviewed By: huntie

Differential Revision: D49190626
cortinico added a commit to cortinico/react-native that referenced this pull request Sep 13, 2023
Summary:

`yarn android` on main is currently broken due to the CLI attempting to search for the manifest inside the src/androidTest folder.
This fixes it by specifying the exact path of the Android Manifest.
The fix to the CLI is also pending to prevent the CLI from searching inside test folders.

Fix for the CLI is here:
- react-native-community/cli#2075

Changelog:
[Internal] [Changed] - Unblock `yarn android` on main

Reviewed By: huntie

Differential Revision: D49190626
@thymikee
Copy link
Member

This exclude list is getting out of hand 😅

@thymikee thymikee merged commit 1ba2916 into react-native-community:main Sep 13, 2023
6 checks passed
@cortinico
Copy link
Member Author

@thymikee I also think that if more than one manifest is found, we should not just pick the first as that's not necessarily the right pick

cortinico added a commit to cortinico/react-native that referenced this pull request Sep 13, 2023
Summary:

`yarn android` on main is currently broken due to the CLI attempting to search for the manifest inside the src/androidTest folder.
This fixes it by specifying the exact path of the Android Manifest.
The fix to the CLI is also pending to prevent the CLI from searching inside test folders.

Fix for the CLI is here:
- react-native-community/cli#2075

Changelog:
[Internal] [Changed] - Unblock `yarn android` on main

Reviewed By: huntie

Differential Revision: D49190626
facebook-github-bot pushed a commit to facebook/react-native that referenced this pull request Sep 13, 2023
Summary:
Pull Request resolved: #39413

`yarn android` on main is currently broken due to the CLI attempting to search for the manifest inside the src/androidTest folder.
This fixes it by specifying the exact path of the Android Manifest.
The fix to the CLI is also pending to prevent the CLI from searching inside test folders.

Fix for the CLI is here:
- react-native-community/cli#2075

Changelog:
[Internal] [Changed] - Unblock `yarn android` on main

Reviewed By: huntie

Differential Revision: D49190626

fbshipit-source-id: 99309f17cb08a33be2a565f5faa29130862686ea
@thymikee
Copy link
Member

Maybe prompting the user which one to pick and adjust the configuration accordingly to avoid prompting again?

@cortinico
Copy link
Member Author

Maybe prompting the user which one to pick and adjust the configuration accordingly to avoid prompting again?

Maybe that's a bit overcomplicated. In general the manifest inside src/main should be preferred.

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.

3 participants