Enable IP tests for `:core` #29202

6hundreds · 2024-05-17T16:41:28Z

This PR is only annotating tests as to-be-fixed-for-ip/incompatible-with-ip and doesn't try to fix them. Although some tests could be fixed easily, this is going to be done in separate PR

bamboo · 2024-05-17T17:11:41Z

...rojects/core/src/integTest/groovy/org/gradle/api/ConfigurationOnDemandIntegrationTest.groovy

@@ -39,7 +40,10 @@ class ConfigurationOnDemandIntegrationTest extends AbstractIntegrationSpec {
        file("gradle.properties") << "org.gradle.configureondemand=true"
    }

-    @Requires(value = IntegTestPreconditions.NotParallelExecutor, reason = "parallel mode hides incubating message")
+    @Requires(
+        value = [IntegTestPreconditions.NotParallelExecutor, IntegTestPreconditions.NotIsolatedProjects],


❓ Since Isolated Projects implies Configuration Cache implies parallel execution, shouldn't NotParallelExecutor be enough here?

Technically speaking, CC executor doesn't imply parallel executor (and was never implying). See

gradle/testing/internal-integ-testing/src/main/groovy/org/gradle/integtests/fixtures/executer/GradleContextualExecuter.java

Line 86 in 3c1f32c

public static boolean isConfigCache() {

CC <-> parallelism relation is being expressed like a

gradle/testing/internal-integ-testing/src/main/groovy/org/gradle/test/preconditions/IntegTestPreconditions.groovy

Line 112 in 6a39fc7

static final class NotParallelOrConfigCacheExecutor implements TestPrecondition {

Interesting. Thanks for the pointers, @6hundreds. I'm now thinking, why shouldn't we update this definition to configCache(true, true)?

@bamboo It could make sense, yes. Let's do that in separate PR, we still can improve test infrastructure for IP, like a #29221

Let's do that in separate PR

Oh, yeah, didn't mean otherwise 👍

bamboo · 2024-05-17T17:12:48Z

...rojects/core/src/integTest/groovy/org/gradle/api/ConfigurationOnDemandIntegrationTest.groovy

@@ -76,6 +83,7 @@ class ConfigurationOnDemandIntegrationTest extends AbstractIntegrationSpec {
        fixture.assertProjectsConfigured(":")
    }

+    @ToBeFixedForIsolatedProjects(because = "Allprojects")


💅 Let's preserve the incompatible API spelling as allprojects for easier searching, just in case

bamboo · 2024-05-17T17:13:35Z

...rojects/core/src/integTest/groovy/org/gradle/api/ConfigurationOnDemandIntegrationTest.groovy

@@ -378,6 +390,7 @@ project(':api') {
        fixture.assertProjectsConfigured(":", ":b", ":a")
    }

+    @ToBeFixedForIsolatedProjects(because = "Configure projects from root, buildDependents is not IP compatible")


💅 Probably better to always keep the API name closer to the because = " for easier search

bamboo · 2024-05-17T17:14:02Z

...rojects/core/src/integTest/groovy/org/gradle/api/ConfigurationOnDemandIntegrationTest.groovy

@@ -378,6 +390,7 @@ project(':api') {
        fixture.assertProjectsConfigured(":", ":b", ":a")
    }

+    @ToBeFixedForIsolatedProjects(because = "Configure projects from root, buildDependents is not IP compatible")


Suggested change

@ToBeFixedForIsolatedProjects(because = "Configure projects from root, buildDependents is not IP compatible")

@ToBeFixedForIsolatedProjects(because = "buildDependents is not IP compatible, configure projects from root")

bamboo · 2024-05-17T17:15:07Z

...gTest/groovy/org/gradle/api/internal/tasks/userinput/UserInputHandlingIntegrationTest.groovy

@@ -150,6 +151,7 @@ class UserInputHandlingIntegrationTest extends AbstractUserInputHandlerIntegrati
        "n"      | _
    }

+    @ToBeFixedForIsolatedProjects(because = "Subprojects")


Suggested change

@ToBeFixedForIsolatedProjects(because = "Subprojects")

@ToBeFixedForIsolatedProjects(because = "subprojects")

bamboo · 2024-05-17T17:18:29Z

...radle/operations/configuration/IsolatedProjectsSettingsBuildOperationsIntegrationTest.groovy


 class IsolatedProjectsSettingsBuildOperationsIntegrationTest extends AbstractIntegrationSpec {

    def operations = new BuildOperationsFixture(executer, temporaryFolder)

+    @Requires(
+        value = IntegTestPreconditions.NotIsolatedProjects,
+        reason = "Exercises disabled IP behavior"


❓ What is the disabled behavior?

Bad wording apparently, sorry. It's better to say that the test is literally controls enablement of IP by itself.

I would suggest moving the annotation to the class level.

In general, I would suspect test classes starting with IsolatedProjects* are either using a corresponding fixture to control the feature, or expect it to not be enabled.

Agree, moved to the top class level

alllex · 2024-05-20T09:24:20Z

.../integTest/groovy/org/gradle/execution/taskgraph/ParallelTaskExecutionIntegrationTest.groovy

@@ -146,6 +147,7 @@ class ParallelTaskExecutionIntegrationTest extends AbstractIntegrationSpec imple
        }
    }

+    @ToBeFixedForIsolatedProjects(because = "Allprojects in setup")


💭 When there is a violation in the setup, would it make sense to put the annotation also on the test class level?

We don't have to change it here, since the work has already been done, but it may be helpful for the annotating other Gradle modules.

Good point.
Now ToBeFixedWithIsolatedProjects is not supposing for type application. It'll require some improvements in the spec extension, so I would plan and do that in a separate PR

bamboo · 2024-05-20T16:43:10Z

...e/src/integTest/groovy/org/gradle/api/tasks/bundling/ConcurrentArchiveIntegrationTest.groovy

@@ -34,6 +34,7 @@ class ConcurrentArchiveIntegrationTest extends AbstractIntegrationSpec {
    BlockingHttpServer server = new BlockingHttpServer()

    @Issue("https://github.com/gradle/gradle/issues/22685")
+    @ToBeFixedForConfigurationCache(because = "Access to root project from sub projects")


🤔 Configuration Cache?

Oooops, my bad

bamboo

🎉

6hundreds added 30 commits May 17, 2024 16:27

Enable IP tests for :core

b7345bf

Fix BuildScriptErrorIntegrationTest

a49976a

Fix ConfigurationOnDemandIntegrationTest

e4d809b

Fix DynamicMethodLookupIntegrationTest

58fda0b

Fix DynamicObjectIntegrationTest

0bded8a

Fix FinalizerTaskIntegrationTest

5c6a7f3

Fix BuildServiceParallelExecutionIntegrationTest

aa002b7

Fix DeferredTaskFailureIntegrationTest

3bcf1e2

Fix DeferredTaskConfigurationIntegrationTest

98a6a1b

Fix ParallelTaskExecutionIntegrationTest

4d3fb9d

Fix TaskSelectionIntegrationTest

756603a

Fix BuildSrcIdentityIntegrationTest

3a542af

Fix ConcurrentArchiveIntegrationTest

1e818a3

Fix ProjectConfigurationIntegrationTest

bcc04fb

Fix CrossBuildScriptCachingIntegrationSpec

2c3bd24

Fix ExternalScriptExecutionIntegrationTest

7d05819

Fix InitScriptExecutionIntegrationTest

0ec8a58

Fix ProducerTaskCommandLineOrderIntegrationTest

8a8a4c0

Fix MultipleTaskOptionsIntegrationTest

0d2af8c

Fix ExecuteUserLifecycleListenerBuildOperationIntegrationTest

d73b935

Fix RuleTaskExecutionIntegrationTest

643b44c

Fix AbstractDirectorySensitivityIntegrationSpec

6c10102

Fix ScriptPluginClassLoadingIntegrationTest

325e22e

Fix IsolatedProjectsSettingsBuildOperationsIntegrationTest

c5c626a

Fix LoggingBuildOperationProgressIntegTest

2d1cdef

Fix BuildSrcVisibilityIntegrationTest

1547e3d

Fix BuildSrcTaskExecutionIntegrationTest

12d0277

Fix RuleTaskBridgingIntegrationTest

3ce918c

Fix TaskSelectorsAndOrdinalNodeIntegrationTest

f88fc47

Fix TaskCreationBuildOperationIntegrationTest

32bfd70

Fix ApplyPluginBuildOperationIntegrationTest

034545d

6hundreds requested review from a team as code owners May 17, 2024 16:41

github-actions bot assigned 6hundreds May 17, 2024

6hundreds requested review from alllex and bamboo and removed request for a team May 17, 2024 16:43

gitstream-cm bot added 10 min review labels May 17, 2024

6hundreds mentioned this pull request May 17, 2024

Start running tests on CI with the Isolated Projects runner #29045

Closed

5 tasks

6hundreds added build-script-change and removed build-script-change labels May 17, 2024

Fix TaskCacheabilityReasonIntegrationTest

ee8cbb3

6hundreds force-pushed the 6hundreds/enable-ip-tests-for-core-ii branch from 1fd897b to ee8cbb3 Compare May 17, 2024 17:01

bamboo requested changes May 17, 2024

View reviewed changes

alllex reviewed May 20, 2024

View reviewed changes

alllex approved these changes May 20, 2024

View reviewed changes

6hundreds added 2 commits May 20, 2024 14:16

Improve API mentioning for better future discovery

43e8fdf

Polish some 'because' statements

659c741

6hundreds force-pushed the 6hundreds/enable-ip-tests-for-core-ii branch from 782a01e to 659c741 Compare May 20, 2024 13:01

bamboo requested changes May 20, 2024

View reviewed changes

bamboo mentioned this pull request May 20, 2024

Add tests for lifecycle callbacks access to surrounding methods #29164

Merged

Fix inaccuracies in tests

1dafea6

bamboo approved these changes May 20, 2024

View reviewed changes

6hundreds added this pull request to the merge queue May 21, 2024

bot-gradle added this to the 8.9 RC1 milestone May 21, 2024

Merged via the queue into master with commit c027624 May 21, 2024
26 checks passed

6hundreds deleted the 6hundreds/enable-ip-tests-for-core-ii branch May 21, 2024 12:59

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Enable IP tests for `:core` #29202

Enable IP tests for `:core` #29202

6hundreds commented May 17, 2024

bamboo May 17, 2024

6hundreds May 20, 2024

bamboo May 20, 2024

6hundreds May 20, 2024

bamboo May 20, 2024

bamboo May 17, 2024

bamboo May 17, 2024

bamboo May 17, 2024

bamboo May 17, 2024

bamboo May 17, 2024

6hundreds May 17, 2024

alllex May 20, 2024

6hundreds May 20, 2024

alllex May 20, 2024

6hundreds May 20, 2024 •

edited

6hundreds May 20, 2024 •

edited

bamboo May 20, 2024

6hundreds May 20, 2024

6hundreds May 20, 2024

bamboo left a comment

	@ToBeFixedForIsolatedProjects(because = "Configure projects from root, buildDependents is not IP compatible")
	@ToBeFixedForIsolatedProjects(because = "buildDependents is not IP compatible, configure projects from root")

Enable IP tests for :core #29202

Enable IP tests for :core #29202

Conversation

6hundreds commented May 17, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

6hundreds May 20, 2024 • edited

Choose a reason for hiding this comment

6hundreds May 20, 2024 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bamboo left a comment

Choose a reason for hiding this comment

Enable IP tests for `:core` #29202

Enable IP tests for `:core` #29202

6hundreds May 20, 2024 •

edited

6hundreds May 20, 2024 •

edited