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

Test.afterTest is not called with Configuration Cache #25311

Closed
TWiStErRob opened this issue Jun 6, 2023 · 11 comments · Fixed by #25983
Closed

Test.afterTest is not called with Configuration Cache #25311

TWiStErRob opened this issue Jun 6, 2023 · 11 comments · Fixed by #25983

Comments

@TWiStErRob
Copy link
Contributor

TWiStErRob commented Jun 6, 2023

Expected Behavior

configurationCache-afterTest_missing>gradlew test --rerun-tasks --configuration-cache
Calculating task graph as no configuration cache is available for tasks: test

> Task :test
com.example.MyTest > myTest: SUCCESS

BUILD SUCCESSFUL in 3s
2 actionable tasks: 2 executed
Configuration cache entry stored.

Current Behavior

configurationCache-afterTest_missing>gradlew test --rerun-tasks --configuration-cache
Calculating task graph as no configuration cache is available for tasks: test

BUILD SUCCESSFUL in 3s
2 actionable tasks: 2 executed
Configuration cache entry stored.

Context (optional)

When I run the same thing without configuration cache:

configurationCache-afterTest_missing>gradlew test --rerun-tasks
> Task :test
com.example.MyTest > myTest: SUCCESS

BUILD SUCCESSFUL in 3s
2 actionable tasks: 2 executed

Originally I had logger.quiet(...) instead of println(), changed it only to make sure there's no captured Project / Task object in play.

Steps to Reproduce

  1. Clone https://github.com/TWiStErRob/repros/tree/main/gradle/configurationCache-afterTest_missing
  2. gradlew test --rerun-tasks (I ran it twice to ensure it always executes)
  3. gradlew test --rerun-tasks --configuration-cache

(Note --rerun-tasks is not necessary for the repro if you delete the build and .gradle folders between executions.)

Gradle version

repro 8.2-rc-1, tried also 8.0.2

Build scan URL (optional)

No response

Your Environment (optional)

I reproduced this on Windows 10 / Java 17. Discovered it inside a Gradle Test Kit test initially, then separated out a minimal repro.

TWiStErRob added a commit to TWiStErRob/repros that referenced this issue Jun 6, 2023
@TWiStErRob
Copy link
Contributor Author

Further testing...

If we add:

test.addTestListener(new MyListener())
class MyListener implements TestListener {
	void beforeSuite(TestDescriptor suite) {
		println("beforeSuite: ${suite}")
	}
	void afterSuite(TestDescriptor suite, TestResult result) {
		println("afterSuite: ${suite} > ${result}")
	}
	void beforeTest(TestDescriptor testDescriptor) {
		println("beforeTest: ${testDescriptor}")
	}
	void afterTest(TestDescriptor testDescriptor, TestResult result) {
		println("afterTest: ${testDescriptor} > ${result}")
	}
}

it outputs this new listener but not the original one:

> Task :test
beforeSuite: Gradle Test Run :test
beforeSuite: Gradle Test Executor 34
beforeSuite: Test class com.example.MyTest
beforeTest: Test myTest(com.example.MyTest)
afterTest: Test myTest(com.example.MyTest) > SUCCESS
afterSuite: Test class com.example.MyTest > SUCCESS
afterSuite: Gradle Test Executor 34 > SUCCESS
afterSuite: Gradle Test Run :test > SUCCESS

If we add

test.addTestListener(new TestListener() {
	void beforeSuite(TestDescriptor suite) {
		println("beforeSuite: ${suite}")
	}
	void afterSuite(TestDescriptor suite, TestResult result) {
		println("afterSuite: ${suite} > ${result}")
	}
	void beforeTest(TestDescriptor testDescriptor) {
		println("beforeTest: ${testDescriptor}")
	}
	void afterTest(TestDescriptor testDescriptor, TestResult result) {
		println("afterTest: ${testDescriptor} > ${result}")
	}
})

then we get expected output, but along with a configuration cache failure:

> Task :test
beforeSuite: Gradle Test Run :test
beforeSuite: Gradle Test Executor 38
beforeSuite: Test class com.example.MyTest
beforeTest: Test myTest(com.example.MyTest)
com.example.MyTest > myTest: SUCCESS
afterTest: Test myTest(com.example.MyTest) > SUCCESS
afterSuite: Test class com.example.MyTest > SUCCESS
afterSuite: Gradle Test Executor 38 > SUCCESS
afterSuite: Gradle Test Run :test > SUCCESS

FAILURE: Build failed with an exception.

* What went wrong:
Configuration cache problems found in this build.

1 problem was found storing the configuration cache.
- Task `:test` of type `org.gradle.api.tasks.testing.Test`: cannot serialize Gradle script object references as these are not supported with the configuration cache.

This is why I think this is rooted in CC.

@bamboo
Copy link
Member

bamboo commented Jun 6, 2023

Related to #24613

@abstratt
Copy link
Member

@TWiStErRob Could you please confirm whether you still see this behavior with tonight's nightly? (builds at midnight)

https://services.gradle.org/distributions-snapshots/

@TWiStErRob
Copy link
Contributor Author

TWiStErRob commented Jun 14, 2023

Still reproduces with https://services.gradle.org/distributions-snapshots/gradle-8.3-20230613222630+0000-all.zip

@abstratt abstratt removed their assignment Jun 22, 2023
@mlopatkin mlopatkin modified the milestones: 8.3 RC1, 8.4 RC1 Jul 4, 2023
@abstratt
Copy link
Member

abstratt commented Aug 29, 2023

@TWiStErRob This time I think we fixed this, when addressing #24613 and friends.

Could you please try with a recent 8.4 build? I saw this working fine after I ran:

./gradlew wrapper --gradle-version=8.4-20230828223511+0000

@TWiStErRob
Copy link
Contributor Author

Based on my repro in OP, it is indeed fixed in the quoted version of yesterday's nightly.

@TWiStErRob
Copy link
Contributor Author

Do you know what fixed it? because the linked #24613's fix only seems to modify TestNG related files.

@abstratt
Copy link
Member

I believe the TestNG references are mostly because we didn't have integration test coverage for registering listeners via AbstractTestTask's (after|before)(Test|Suite) in JUnit-related tests. I am looking into adding that now.

@abstratt abstratt linked a pull request Aug 29, 2023 that will close this issue
@abstratt
Copy link
Member

But actually answering your question, I believe the fix was related to simplifying how theAbstractTestTask state for managing listeners was serialized (#25983).

@TWiStErRob
Copy link
Contributor Author

Thank you!

@abstratt
Copy link
Member

Fixed by #25983.

bot-gradle added a commit that referenced this issue Aug 30, 2023
…in AbstractTestTask

...which were not working before 8.4.

Issue: #25311, #25983

Co-authored-by: Rafael Chaves <rchaves@gradle.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants