Skip to content

[Infra] Test suites #18

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 7 commits into from
May 26, 2020
Merged

[Infra] Test suites #18

merged 7 commits into from
May 26, 2020

Conversation

Leo-Neat
Copy link

Add infrastructure for test suites. It allows for tests to be grouped together an run. The basic, auto, and automotive test suites have been added.

@Leo-Neat Leo-Neat requested a review from nic0lette May 20, 2020 17:57
@Leo-Neat
Copy link
Author

@woshizhangchen

nic0lette
nic0lette approved these changes May 20, 2020
View reviewed changes
Copy link
Member

@nic0lette nic0lette left a comment

Choose a reason for hiding this comment

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

LGTM with a few tiny nits.

mediacontroller/src/main/java/com/example/android/mediacontroller/MediaAppTestingActivity.kt Outdated
import android.view.MenuItem
import android.view.View
import android.view.ViewGroup
import android.view.*
Copy link
Member

Choose a reason for hiding this comment

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

nit: Sorry, but could you not use the * imports? :)

Copy link
Author

Choose a reason for hiding this comment

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

Ya sorry about the repeating issue. The formatted in android studio auto does that and sometimes I forget to undo it. But I will be on the lookout now!

mediacontroller/src/main/res/layout/media_test_suite_result.xml Outdated
@@ -1,5 +1,4 @@

nit: Could you move the "
Copy link
Author

Choose a reason for hiding this comment

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

Another auto formatting thing. I will be on the lookout!

mediacontroller/src/main/java/com/example/android/mediacontroller/MediaAppTestDetails.kt Outdated
@@ -29,15 +29,13 @@ import android.support.v4.media.session.MediaControllerCompat
import android.support.v4.media.session.PlaybackStateCompat
import android.util.Log
import android.util.TypedValue
import android.widget.TextView
import androidx.annotation.RequiresApi
import com.example.android.mediacontroller.Test.Companion.androidResources
import java.text.DateFormat
import java.util.*
Copy link
Member

Choose a reason for hiding this comment

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

nit: I know this wasn't you, but would you mind expanding the java.util.* import since you're updating this file?

Copy link
Author

Choose a reason for hiding this comment

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

Done.

nic0lette
nic0lette reviewed May 20, 2020
View reviewed changes
mediacontroller/src/main/res/layout/test_suite_results_dialog.xml
@@ -0,0 +1,52 @@
Copy link
Member

Choose a reason for hiding this comment

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

Oh, Apache license header should be included here.

Copy link
Author

Choose a reason for hiding this comment

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

Done.

nic0lette
nic0lette reviewed May 20, 2020
View reviewed changes
mediacontroller/src/main/java/com/example/android/mediacontroller/MediaAppTestSuite.kt
@@ -0,0 +1,198 @@
package com.example.android.mediacontroller
Copy link
Member

Choose a reason for hiding this comment

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

License header x.x Sorry I missed this the first time.

Copy link
Author

Choose a reason for hiding this comment

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

Done.

woshizhangchen
woshizhangchen reviewed May 26, 2020
View reviewed changes
mediacontroller/src/main/java/com/example/android/mediacontroller/MediaAppTestDetails.kt Outdated
@@ -50,6 +52,7 @@ class Test(
) : HandlerThread(testName) {
private val steps = mutableListOf()
private var stepIndex = 0
private val TAG = "MediaAppTestDetails"
Copy link

Choose a reason for hiding this comment

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

maybe put the tag as the first variable?

Copy link
Author

Choose a reason for hiding this comment

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

Done.

woshizhangchen
woshizhangchen reviewed May 26, 2020
View reviewed changes
mediacontroller/src/main/java/com/example/android/mediacontroller/MediaAppTestSuite.kt Outdated
}


override fun onClick(p0: View?) {
Copy link

Choose a reason for hiding this comment

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

empty method?

Copy link
Author

Choose a reason for hiding this comment

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

Removed. Good catch.

woshizhangchen
woshizhangchen reviewed May 26, 2020
View reviewed changes
mediacontroller/src/main/java/com/example/android/mediacontroller/MediaAppTestDetails.kt Outdated
@@ -184,6 +187,7 @@ class Test(
}

callback = object : MediaControllerCompat.Callback() {

Copy link

Choose a reason for hiding this comment

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

nit:extra line

Copy link

Choose a reason for hiding this comment

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

seems to be a few other places in the file with extra lines

Copy link
Author

Choose a reason for hiding this comment

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

I think there needs to be an extra like here for formatting but I found other places in the file where they were not necessary.

woshizhangchen
woshizhangchen reviewed May 26, 2020
View reviewed changes
Copy link

@woshizhangchen woshizhangchen left a comment

Choose a reason for hiding this comment

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

lgtm

@Leo-Neat Leo-Neat merged commit 5525c7f into googlesamples:master May 26, 2020
@Leo-Neat Leo-Neat deleted the test-suites branch May 26, 2020 15:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nic0lette nic0lette nic0lette approved these changes

@woshizhangchen woshizhangchen woshizhangchen left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@Leo-Neat @nic0lette @woshizhangchen