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

Continuous testing gets messed up when stashing/unstashing changesets that have changes to tests #28376

Closed
edeandrea opened this issue Oct 4, 2022 · 13 comments · Fixed by #29262
Labels
Milestone

Comments

@edeandrea
Copy link
Contributor

edeandrea commented Oct 4, 2022

Describe the bug

When continuous testing is turned on, I notice that if I stash (or unstash) a change set that contains changes to the source code and/or test code, continuous testing gets messed up. I have to quit dev mode and re-run it in order for it to then work right again.

I recorded a video of the behavior (I couldn't attach it because it was too big, so I put it on YouTube):
https://youtu.be/oDd93zDj4Is

Its almost like continuous testing doesn't see the changes to the tests (but it does the main source set) and therefore the tests no longer compile (even though they should).

Here is the entire console output from when the change set is stashed (it was too long for GitHub to allow me to post it, so I put it in a .txt file):
continuous testing error after stashing.txt

Expected behavior

I would expect after stashing/unstashing changes that continuous testing would pick up the new changes.

Actual behavior

Continuous testing gets messed up somehow and I need to completely quit dev mode and restart it in order for it to continue working properly.

How to Reproduce?

I was using the Hero service from the Quarkus superheroes application.

Here is a patch file containing the change set I had applied on top of the super heroes main branch:
Changes_to_powers_-_hero_service.txt

I applied that patch then started dev mode & turned on continuous testing. Then I stashed/shelved that patch and boom.

Output of uname -a or ver

Darwin edeandrea-m1pro 21.6.0 Darwin Kernel Version 21.6.0: Mon Aug 22 20:19:52 PDT 2022; root:xnu-8020.140.49~2/RELEASE_ARM64_T6000 arm64

Output of java -version

openjdk version "17.0.4" 2022-07-19
OpenJDK Runtime Environment Temurin-17.0.4+8 (build 17.0.4+8)
OpenJDK 64-Bit Server VM Temurin-17.0.4+8 (build 17.0.4+8, mixed mode)

GraalVM version (if different from Java)

No response

Quarkus version or git rev

2.13.0.Final

Build tool (ie. output of mvnw --version or gradlew --version)

Apache Maven 3.8.4 (9b656c72d54e5bacbed989b64718c159fe39b537)
Maven home: /Users/edeandre/.m2/wrapper/dists/apache-maven-3.8.4-bin/52ccbt68d252mdldqsfsn03jlf/apache-maven-3.8.4
Java version: 17.0.4, vendor: Eclipse Adoptium, runtime: /Users/edeandre/.sdkman/candidates/java/17.0.4-tem
Default locale: en_US, platform encoding: UTF-8
OS name: "mac os x", version: "12.6", arch: "aarch64", family: "mac"

Additional information

No response

@edeandrea edeandrea added the kind/bug Something isn't working label Oct 4, 2022
@quarkus-bot
Copy link

quarkus-bot bot commented Oct 4, 2022

/cc @stuartwdouglas

@edeandrea
Copy link
Contributor Author

Any thoughts anyone?

@edeandrea
Copy link
Contributor Author

Any thoughts to what this might be? @geoand ?

@geoand
Copy link
Contributor

geoand commented Nov 2, 2022

Not really unfortunately

@edeandrea
Copy link
Contributor Author

Is there anyone who can take a look at this?

When I'm demoing Quarkus and I have some changes "pre-baked" as a stashed commit, I have to stop the Quarkus app, unstash my changes, then restart my Quarkus app.

I've had people ask me "how come you have to restart dev mode & continuous testing? I though live coding meant you never had to restart it?"

If it helps I could probably put together a more trivial reproducer.

@stuartwdouglas
Copy link
Member

Can you test out the linked PR?

gsmet pushed a commit to stuartwdouglas/quarkus that referenced this issue Nov 15, 2022
@edeandrea
Copy link
Contributor Author

edeandrea commented Nov 15, 2022

Hi @stuartwdouglas - the linked PR does not solve the problem (unless I did something wrong...)

This is what I did to test it:

  1. Clone quarkusio/quarkus
  2. cd into quarkus
  3. gh pr checkout 29262
  4. ./mvnw -Dquickly
  5. Back in rest-heroes, run quarkus dev --clean -Dquarkus.platform.group-id=io.quarkus -Dquarkus.platform.version=999-SNAPSHOT
  6. Turn on continuous testing
  7. Follow same steps as I did in https://youtu.be/oDd93zDj4Is
  8. Got same result.

@quarkus-bot quarkus-bot bot added this to the 2.15 - main milestone Nov 15, 2022
@edeandrea
Copy link
Contributor Author

Hi @gsmet - why did you close this issue? The PR which was linked does not solve the issue...

@gsmet gsmet modified the milestones: 2.15 - main, 2.14.1.Final Nov 15, 2022
gsmet pushed a commit to gsmet/quarkus that referenced this issue Nov 15, 2022
@stuartwdouglas
Copy link
Member

So the remaining issue is actually an annotation processor/mapstruct issue, basically a duplicate of #1502.

@stuartwdouglas
Copy link
Member

I wonder if there is an automated way to solve this. It might be a bit involved but basically:

  • Load all annotation processors and get the list of supported annotations.
  • Examine all classes with one of these annotations, parse the class with ASM and get a list of referenced classes from the bytecode const pool.
  • If a referenced class is changed, then also recompile the class that is an annotation processor target.

@edeandrea
Copy link
Contributor Author

So the remaining issue is actually an annotation processor/mapstruct issue, basically a duplicate of #1502.

Interesting. Honestly I didn't even think about that, nor did I realize that annotation processors weren't picked up/re-run in dev mode.

How does dev mode work with Panache? isn't the quarkus-panache-common defined as an annotation processor?

@stuartwdouglas
Copy link
Member

Annotation processors work fine if the class being processed is the class being changed. For panache it works on the entity itself, if you change the entity the entity is recompiled and the processor is run.

In this case the Hero is changed but HeroMapper is not, and HeroMapper is the class that the annotation processor runs against. Modify the mappers as well would result in this working.

The only way we could fix this would be to try and expand the list of files to recompile, either by having an extension provide this information, or some kind of best effort automated approach like I have proposed above.

@edeandrea
Copy link
Contributor Author

or some kind of best effort automated approach like I have proposed above.

Wouldn't that be better than what is there now?

@gsmet gsmet modified the milestones: 2.14.1.Final, 2.13.6.Final Dec 14, 2022
gsmet pushed a commit to gsmet/quarkus that referenced this issue Dec 14, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants