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

Add tests #151

Merged
merged 4 commits into from
Sep 22, 2020
Merged

Add tests #151

merged 4 commits into from
Sep 22, 2020

Conversation

deepueg
Copy link
Contributor

@deepueg deepueg commented Sep 8, 2020

No description provided.

Copy link
Member

@friederbluemle friederbluemle left a comment

Choose a reason for hiding this comment

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

Changes look good overall. The Upgrade gradle commit was missing some updates to the wrapper scripts - something that the wrapper task does (Android Studio won't do it). I opened a quick PR with only those changes here: #152
So after rebasing, you can exclude the file from the commit, and also reword it to Update Android Gradle plugin to 4.0.1

Bundle props = getDefaultProps();
addGlobalProps(props);
if (data != null) {
props.putAll(data);
}
mMiniAppView.setAppProperties(props);
((ReactRootView)mMiniAppView).setAppProperties(props);
Copy link
Member

Choose a reason for hiding this comment

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

Missing space after cast

@@ -16,6 +16,9 @@ android {
minifyEnabled false
proguardFiles getDefaultProguardFile('proguard-android.txt'), 'proguard-rules.pro'
}
debug {
multiDexEnabled true
Copy link
Member

Choose a reason for hiding this comment

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

Is this required..?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, required for instrumentation testing

Copy link
Member

Choose a reason for hiding this comment

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

But this enables it not only for testing... Why is it required for instrumentation testing?


@Test
public void testRootComponentRendering() {
Assert.assertNotNull(rule);
Copy link
Member

Choose a reason for hiding this comment

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

Best practice to use static imports for things like assertNotNull etc. (Just like you've already added a static import for fail) - It makes these lines shorter and easier to read.

androidTestImplementation 'androidx.test.ext:junit:1.1.1'
androidTestImplementation 'androidx.test.espresso:espresso-core:3.2.0'
androidTestImplementation 'androidx.swiperefreshlayout:swiperefreshlayout:1.1.0'
androidTestImplementation 'com.android.support:multidex:1.0.3'
Copy link
Contributor

Choose a reason for hiding this comment

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

you can use "androidx.multidex:multidex:2.0.1" instead

@deepueg deepueg force-pushed the add-tests branch 2 times, most recently from 89b64f0 to 5591e00 Compare September 22, 2020 18:52
@deepueg deepueg merged commit ce6fe32 into electrode-io:master Sep 22, 2020
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