Skip to content

Conversation

@jonsimantov
Copy link
Contributor

Added integration test build for Desktop and Android. iOS requires adding dependencies manually right now and so does not yet build automatically.

Tested on Mac and Linux for both desktop and Android. Not yet tested on Windows, but will do that in a follow-up after we hook it up to the GHA.

Note that there are a few manual things that have to happen before the integration test can be run:

  • You must put the google-services.json file into the directory (or GoogleService-Info.plist, for iOS). This should be done via github secrets, but is not included in this CL.
  • For dynamic_links or messaging, you must edit src/integration_test.cc and replace the string that says "REPLACE_..."

@jonsimantov jonsimantov requested review from vimanyu and removed request for DellaBitta July 9, 2020 18:19
@jonsimantov
Copy link
Contributor Author

I don't know why all the stuff from the merge ended up in this PR or how to get rid of it...

@jonsimantov jonsimantov force-pushed the feature/js-integration-tests branch from 1f1dcd0 to d68b186 Compare July 10, 2020 03:54
@jonsimantov
Copy link
Contributor Author

I had to squash this into a single commit to fix the issue with the merge commit being included. Sorry about that!

@jonsimantov jonsimantov force-pushed the feature/js-integration-tests branch from d68b186 to bdcd3f3 Compare July 10, 2020 04:01
Copy link
Contributor

@DellaBitta DellaBitta left a comment

Choose a reason for hiding this comment

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

Mainly Copyright police. Some of my copyright comments have ?s -- while the AndroidManifest.xml should have copyrights (if we're following the other google open source projects) I wasn't sure if the strings.xml and main.xml should. I would think so...

@anonymous-akorn
Copy link
Contributor

anonymous-akorn commented Jul 14, 2020

It looks like the "(api)/integration_test/src" directories are missing all the *_framework files. Internally they're symlinks, but it looks like they didn't get included in this PR in any form.

Edit: Looks like other symlinks are missing as well, such as download_googletest.py.

@jonsimantov
Copy link
Contributor Author

It looks like the "(api)/integration_test/src" directories are missing all the *_framework files. Internally they're symlinks, but it looks like they didn't get included in this PR in any form.

Edit: Looks like other symlinks are missing as well, such as download_googletest.py.

download_googletest.py and the symlinks in src are populated by the setup_integration_tests.py script, which is run by the build process in each mode of building (Podfile, build.gradle, and cmake file) before downloading googletest.

@jonsimantov
Copy link
Contributor Author

Mainly Copyright police. Some of my copyright comments have ?s -- while the AndroidManifest.xml should have copyrights (if we're following the other google open source projects) I wasn't sure if the strings.xml and main.xml should. I would think so...

No harm in adding copyright to as many files as possible - I added it to all xml, bat, gradlew, and build.gradle files.

@jonsimantov jonsimantov reopened this Jul 15, 2020
@jonsimantov
Copy link
Contributor Author

Added copyrights and fixed the Info.plist files, PTAL.

@anonymous-akorn
Copy link
Contributor

It looks like the "(api)/integration_test/src" directories are missing all the *_framework files. Internally they're symlinks, but it looks like they didn't get included in this PR in any form.
Edit: Looks like other symlinks are missing as well, such as download_googletest.py.

download_googletest.py and the symlinks in src are populated by the setup_integration_tests.py script, which is run by the build process in each mode of building (Podfile, build.gradle, and cmake file) before downloading googletest.

Ah, I see why it's not running for me - the testapp builder copies the integration test project to another directory to work clean. The location of the setup script is determined relative to the cmake file in the test project, and only tries to run if found. Could be fixed by looking relative to the SDK/repo directory instead, but I'm not sure how easy it is to get a reference to that in each build file (e.g. from the pod file).

It's not difficult to work around if this is intended behaviour, though - I could change the testapp builder to copy the whole repo instead of just the integration test project, as it's not very large (~50mb).

@jonsimantov
Copy link
Contributor Author

It looks like the "(api)/integration_test/src" directories are missing all the *_framework files. Internally they're symlinks, but it looks like they didn't get included in this PR in any form.
Edit: Looks like other symlinks are missing as well, such as download_googletest.py.

download_googletest.py and the symlinks in src are populated by the setup_integration_tests.py script, which is run by the build process in each mode of building (Podfile, build.gradle, and cmake file) before downloading googletest.

Ah, I see why it's not running for me - the testapp builder copies the integration test project to another directory to work clean. The location of the setup script is determined relative to the cmake file in the test project, and only tries to run if found. Could be fixed by looking relative to the SDK/repo directory instead, but I'm not sure how easy it is to get a reference to that in each build file (e.g. from the pod file).

It's not difficult to work around if this is intended behaviour, though - I could change the testapp builder to copy the whole repo instead of just the integration test project, as it's not very large (~50mb).

It's intended - I think the best way to do it might be to run the setup script, and then copy the integration tests to a working directory.

@jonsimantov
Copy link
Contributor Author

(Also that would only be necessary when running the integration tests against a binary SDK. You can run the integration tests against the source SDK directly inside the repo.)

@jonsimantov
Copy link
Contributor Author

@a-maurice @vimanyu @DellaBitta ping?

@jonsimantov jonsimantov merged commit fb3b0df into dev Jul 17, 2020
@jonsimantov jonsimantov deleted the feature/js-integration-tests branch July 22, 2020 23:11
@firebase firebase locked and limited conversation to collaborators Aug 17, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants