-
Notifications
You must be signed in to change notification settings - Fork 1.3k
Integration tests suddenly not working #1336
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
Comments
What changed? Did it break when you bumped the Compose version or something else? Do you have a minimized/sharable sample which gets it into this state? It is going to be hard to debug without a simplified/sharable reproducer. cc @theapache64 Looks like the OP is using your template. Have you upgraded the template to the latest Compose MPP release by chance? Have you ever run into this error? |
I didn't use the template, just pulled in some of the dependencies I saw in there. I guess it did fail after one of the upgrades, I think we upgraded GP? I have a repo, I can add someone to it. It's private right now. |
@jimgoog I've never seen this error before. It is hard to identify the issue without the code. |
Here's the test:
Here's the gradle file:
|
I looked at the code for the
Well seems pretty clear, no? we never get inside the evaluate() or don't get there before the first assertion is made in the test, which asks for the root and fails. Sure enough a breakpoint placed at the assignment of Guess the tests for the tests didn't work, eh? |
Wow, this is bad. Minimal repro looks like:
How did we miss this? Are we still not running desktop tests in Google CI? Seems should turn that on. cc @igordmn can you take a look? |
We run them on our CI, all tests pass.
That would be great, but there are a couple of tests which are failing in
The issue seems old, as
|
It seems I was wrong. I have checked beta5, and tests work, even if we use I used
My guess, you mix junit5 and junit4. Try to remove
by
|
The two changes resulted in runner finding no tests. Maybe have to comment the 5 runner too? lemme try that... |
That worked:
Thanks! |
Kind of insane that we can't use JUnit 5 but should maybe add that to the readme. |
We use junit4 internally and you imported org.jetbrains.compose.ui:ui-test-junit4-desktop as a dependency presumably to reuse the test rules we use instead of writing your own. There is nothing stopping you from writing a junit5 test rule. We can't support every version of every test library, although it does seem likely we would add junit5 at some point in the near-ish future. Certainly I think we would take community contributions to add such support. In the meantime, the error above is rather surprising / non-obvious. Might be worth adding something that gives a more sensible error message if the user is running a junit4 test with a junit5 engine. It's unfortunate that junit doesn't automatically detect such misconfigurations and give helpful messages. |
Yeah sorry if my redux was dismissive, I am down on JUnit in general. 5 doesn't really add that much, we only use it to be on a chain that is under active dev, 4 was released 15 years ago. Is that all this was though? I also wonder why the test was working when we first did it. We put the 5 runner in when we first made the project so there's no way our original runs were with 4. Totally agree that the error is as the Pauli quote goes: 'not even wrong.' |
Yeah, we will add this to readme in #368.
It is probably not trivial on the user side. And there are a couple of things that are marked as
We can add an assert to all public methods with the message:
JUnit 5 can't use |
Taking into consideration that JUnit 4 is 16 years old and JUnit 5 already 5 years old, I would say that official support for the latest major version of the industry standard is way overdue. Of course, the sheer majority of the blame falls into Google for their lack of JUnit 5 support for Android testing.
It's not really that difficult. I'm sure there will be compromises, but simply copying the two classes from mannodermaus/android-junit5#234 (comment) allowed me to run a JUnit5 UI test in Compose for Desktop.
|
Closing this in favor to more specific issues: |
Please check the following ticket on YouTrack for follow-ups to this issue. GitHub issues will be closed in the coming weeks. |
Uh oh!
There was an error while loading. Please reload this page.
We had them working after reading through this article (and adding some android classes):
But then we ran into a situation where it was complaining that the lateinit window was never initialized. We tried wrapping the content in a Window {} (both the new version and the deprecated one). Did not work. Then we upgraded to the latest. Now it says scene not found.
The text was updated successfully, but these errors were encountered: