Manifest merger fails when assembling Android test application

[sonicdoe]

Description

When assembling the Android test application using ./gradlew assembleAndroidTest, the manifest merger fails with the following errors:

> Task :react-native-onesignal:processDebugAndroidTestManifest FAILED
/…/react-native-onesignal/examples/RNOneSignal/node_modules/react-native-onesignal/android/build/intermediates/tmp/manifest/androidTest/debug/manifestMerger8422387923641009676.xml Error:
        Attribute meta-data#onesignal_app_id@value at manifestMerger8422387923641009676.xml requires a placeholder substitution but no value for <onesignal_app_id> is provided.
/…/react-native-onesignal/examples/RNOneSignal/node_modules/react-native-onesignal/android/build/intermediates/tmp/manifest/androidTest/debug/manifestMerger8422387923641009676.xml Error:
        Attribute meta-data#onesignal_google_project_number@value at manifestMerger8422387923641009676.xml requires a placeholder substitution but no value for <onesignal_google_project_number> is provided.

See http://g.co/androidstudio/manifest-merger for more information about the manifest merger.

A similar issue was reported with #579 in July 2018 and subsequently closed in January 2019 without any apparent resolution.

Environment

The example project at examples/RNOneSignal@31b5242 which uses react-native-onesignal v3.2.14.

Steps to reproduce

1. git clone https://github.com/geektimecoil/react-native-onesignal.git
2. cd react-native-onesignal
3. git checkout 31b52420c807c22405e70b44800e20f3a8e3456e
4. cd examples/RNOneSignal
5. yarn
6. cd android
7. ./gradlew assembleAndroidTest

[wkoutre]

Running into this, as well.

[wkoutre]

@sonicdoe Are you using productFlavors? If so, you'll need to change the command like:

buildType = "debug"
productFlavor = "dev"

./gradlew assembleDevDebugAndroidTest

[sonicdoe]

No, the example project at examples/RNOneSignal@31b5242 (which I’ve used to reproduce this) does not use product flavors.

[dorthwein]

I'm running into this as well - a random google search showed this issue crosswalk-project/cordova-plugin-crosswalk-webview#207 which suggests using buildToolsVersion: 28.0.0.

We already do this, but then I noticed a warning during build

OneSignalPlugin: WARNING: OneSignalPlugin: Downgraded 'com.android.support:28.0.0' -> 27.+ to prevent compile errors! Recommend updating your project's compileSdkVersion!

where this project is forcing it to downgrade ;(

[wkoutre]

My personal issue was resolved when I built using the proper task-name after the gradlew command (described above).

FWIW, here is part of my android/build.gradle file:

subprojects {
    afterEvaluate {project ->
        if (project.hasProperty("android")) {
            android {
                compileSdkVersion 28
                buildToolsVersion '28.0.3'
            }
        }
    }
    project.configurations.all {
        resolutionStrategy.eachDependency { details ->
            if (details.requested.group == 'com.android.support'
                    && !details.requested.name.contains('multidex') ) {
                details.useVersion "28.0.0"
            }
        }
    }
}

[rgomezp]

Thanks for your patience while we investigate the issue.

[rgomezp]

Howdy y'all,
We're not currently sure how to permanently fix this. In the meantime, you can run:
./gradlew app:assembleAndroidTest

We also realized we were running into issues with ./gradlew build so we are creating a PR that will fix this too.

Thanks for bringing this to our attention.

[sonicdoe]

I was able to get assembleAndroidTest to work by injecting the manifest placeholders into react-native-onesignal itself (similar to #579 (comment)) and removing the ApplicationTest class in android/src/androidTest (which seems to have no effect anyway). See my gradle-android-test branch for all changes.

Unfortunately, I couldn’t get to the bottom of why this issue occurs and why these changes are necessary. As far as I could find out, androidTest resolves to a different dependency tree which is why the manifest merger behaves differently in that case.

@rgomezp Let me know if you have any more insight and whether the above changes would be okay. If so, I’d open a pull request.

[rgomezp]

@sonicdoe ,
Thanks for the fix @sonicdoe! Please create the PR

[mtt87]

@rgomezp adding my question here since you closed the other ticket

is this directory necessary to be included in the npm package or can it only live in the GitHub repo?

Because if the answer is no, then this problem is solved for me.

I'm asking because I don't know what are good practices but I don't see other RN packages with this directory androidTest so I guess either nobody is testing or they don't include tests in the npm package?

[rgomezp]

@mtt87 ,
It isn't necessary. The original creators must have accidentally included it. We are removing it in the upcoming RN OneSignal SDK release. Thanks!

[mtt87]

Fantastic 😄 Thank you

[rgomezp]

Howdy y'all,
We will be releasing the newest version of the react-native SDK with fixes very soon. If you would like to test the changes now, simply change your react-native-onesignal in your package.json to point to this repository.

"react-native-onesignal":"geektimecoil/react-native-onesignal"

Would love to hear your feedback prior to the release. Cheers!

EDIT: IGNORE THESE INSTRUCTIONS AS THE PLUGIN HAS BEEN PUSHED TO NPM

[sonicdoe]

I see you’ve already released the newest version, v3.3.0. Should this release change anything about this issue? If I repeat the reproduction steps from the original post, I’m running into the exact same error.

[rgomezp]

@sonicdoe ,
Are you sure you reinstalled it correctly?

Edit: just tested. You are correct. My apologies here. I thought #791 fixed this. I will discuss with @jkasten2 regarding #789