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

Abstract @Test method #13

Closed
bootstraponline opened this issue May 4, 2018 · 7 comments
Closed

Abstract @Test method #13

bootstraponline opened this issue May 4, 2018 · 7 comments

Comments

@bootstraponline
Copy link

I think it's a bug that this test is detected as valid.

public abstract class TeacherTest {

    @Test
    public abstract void displaysPageObjects();
}

java.lang.InstantiationException: Can't instantiate abstract class com.instructure.teacher.ui.utils.TeacherTest

@drewhannay
Copy link
Contributor

The subject says Kotlin, but the code snippet is Java code. It looks like this is an issue in both languages though. I'm not sure if there's any valid reason to have @Test on an abstract method in JUnit, but I'll do some research.

I probably won't have time to take a look at this until after Google I/O next week, but I'll follow up.

@bootstraponline
Copy link
Author

I'm at I/O this week as well. We're using dex-test-parse to power Flank. It'd be cool to meet up if our schedules align. I'm on the firebase slack if you want to chat.

https://firebase.community

I worked around this issue by removing the annotation.

@bootstraponline bootstraponline changed the title Kotlin abstract class @Test abstract test method Abstract @Test method May 7, 2018
@kkoser
Copy link
Contributor

kkoser commented Jun 21, 2018

I just did some testing on this in a test project, and it looks like adb instrument with the Junit4 runner DOES notice and run implementations of abstract methods that were annotated with @test, even if the concrete implementation of that method does not. So it's probably also something dex-test-parser should support, although I'm having trouble thinking of a good reason to use this functionality...

@bootstraponline
Copy link
Author

The issue is causing problems for Flank.

For Firebase Test Lab, we need the ability to filter out the abstract classes.

@bootstraponline
Copy link
Author

If I run this text locally or directly with Firebase (without using Flank) the list of tests executed will be:
espresso.fail.multidex.FooTest#testBar
espresso.fail.multidex.FooTest#testMainActivity
espresso.fail.multidex.FooTest#testAbstractBar
espresso.fail.multidex.FooTest#testAbstractBar2

but if I use Flank I will get:
espresso.fail.multidex.FooTest#testBar
espresso.fail.multidex.FooTest#testMainActivity
espresso.fail.multidex.AbstractFooTest#testAbstractBar
espresso.fail.multidex.AbstractFooTest#testAbstractBar2

that seems like a valid use case? I'm not sure there's an easy way to resolve methods in the same way.

@kkoser
Copy link
Contributor

kkoser commented Jul 19, 2018

See #16 for a solution

drewhannay pushed a commit that referenced this issue Jul 23, 2018
* Add support for test classes that extend other test classes

This supports both abstract and concrete superclasses.

Normally, test methods are found via reflection which is why methods defined from the super class would be included. This
adds the same behavior to dex-test-parser to match with what is expected when running tests via adb directly.

* Fix test to include the new abstract test methods in expectation

* Fix naming, remove superfluous logs

* Bump version to 2.0

This is a breaking change to the methods to find JUnit4 methods. Also updated the changelog to indicate this

* Update changelog styling

We want to have a section for the current snapshot, so people can add changes there as they are added
@drewhannay
Copy link
Contributor

Fixed by #16

Note that this is only fixed for JUnit 4 tests, not JUnit 3. Since Google has deprecated the JUnit 3 base classes in the Android SDK and is now recommending that new tests should be written with the Android Testing Support Library (which is JUnit 4), it's extremely unlikely we'll fix this issue for JUnit 3 (it's not a very straightforward change at all, and would likely have a negative impact on the runtime performance of dex-test-parser).

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

No branches or pull requests

3 participants