You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@geode.apache.org by GitBox <gi...@apache.org> on 2020/10/21 19:21:56 UTC
[GitHub] [geode] demery-pivotal opened a new pull request #5649: GEODE-8637: Give each test worker a unique working dir
demery-pivotal opened a new pull request #5649:
URL: https://github.com/apache/geode/pull/5649
Before running a test task, wrap its TestFramework in a wrapper that creates a unique working dir for each new test worker.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [geode] demery-pivotal commented on a change in pull request #5649: GEODE-8637: Give each test worker a unique working dir
Posted by GitBox <gi...@apache.org>.
demery-pivotal commented on a change in pull request #5649:
URL: https://github.com/apache/geode/pull/5649#discussion_r509733062
##########
File path: buildSrc/src/main/java/org/apache/geode/gradle/RunInSubdirectoryTestFramework.java
##########
@@ -0,0 +1,102 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more contributor license
+ * agreements. See the NOTICE file distributed with this work for additional information regarding
+ * copyright ownership. The ASF licenses this file to You under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the License. You may obtain a
+ * copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software distributed under the License
+ * is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express
+ * or implied. See the License for the specific language governing permissions and limitations under
+ * the License.
+ */
+
+package org.apache.geode.gradle;
+
+import static java.nio.file.StandardCopyOption.COPY_ATTRIBUTES;
+
+import java.io.IOException;
+import java.io.UncheckedIOException;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.util.concurrent.atomic.AtomicLong;
+
+import org.gradle.api.Action;
+import org.gradle.api.internal.tasks.testing.TestFramework;
+import org.gradle.api.internal.tasks.testing.WorkerTestClassProcessorFactory;
+import org.gradle.api.internal.tasks.testing.detection.TestFrameworkDetector;
+import org.gradle.api.tasks.testing.TestFrameworkOptions;
+import org.gradle.process.internal.JavaExecHandleBuilder;
+import org.gradle.process.internal.worker.WorkerProcessBuilder;
+
+/**
+ * Wraps a test framework to run each test worker in a separate working directory.
+ */
+public class RunInSubdirectoryTestFramework implements TestFramework {
+ private static final String GEMFIRE_PROPERTIES = "gemfire.properties";
+ private final AtomicLong workerId = new AtomicLong();
+ private final TestFramework delegate;
+
+ public RunInSubdirectoryTestFramework(TestFramework delegate) {
+ this.delegate = delegate;
+ }
+
+ @Override
+ public TestFrameworkDetector getDetector() {
+ return delegate.getDetector();
+ }
+
+ @Override
+ public TestFrameworkOptions getOptions() {
+ return delegate.getOptions();
+ }
+
+ @Override
+ public WorkerTestClassProcessorFactory getProcessorFactory() {
+ return delegate.getProcessorFactory();
+ }
+
+ /**
+ * Return an action that configures the test worker builder to run the test worker in a unique
+ * subdirectory of the task's working directory.
+ */
+ @Override
+ public Action<WorkerProcessBuilder> getWorkerConfigurationAction() {
+ return workerProcessBuilder -> {
+ delegate.getWorkerConfigurationAction().execute(workerProcessBuilder);
+ JavaExecHandleBuilder javaCommand = workerProcessBuilder.getJavaCommand();
+
+ Path taskWorkingDir = javaCommand.getWorkingDir().toPath();
+ String workerWorkingDirName = String.format("test-worker-%06d", workerId.incrementAndGet());
+ Path workerWorkingDir = taskWorkingDir.resolve(workerWorkingDirName);
+
+ createWorkingDir(workerWorkingDir);
+ copyGemFirePropertiesFile(taskWorkingDir, workerWorkingDir);
Review comment:
Do you have a reason to believe it's still needed? If you're not sure, I can do a run without it and see what explodes.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [geode] demery-pivotal commented on a change in pull request #5649: GEODE-8637: Give each test worker a unique working dir
Posted by GitBox <gi...@apache.org>.
demery-pivotal commented on a change in pull request #5649:
URL: https://github.com/apache/geode/pull/5649#discussion_r509714692
##########
File path: buildSrc/src/main/java/org/apache/geode/gradle/RunInSubdirectoryTestFramework.java
##########
@@ -0,0 +1,102 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more contributor license
+ * agreements. See the NOTICE file distributed with this work for additional information regarding
+ * copyright ownership. The ASF licenses this file to You under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the License. You may obtain a
+ * copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software distributed under the License
+ * is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express
+ * or implied. See the License for the specific language governing permissions and limitations under
+ * the License.
+ */
+
+package org.apache.geode.gradle;
+
+import static java.nio.file.StandardCopyOption.COPY_ATTRIBUTES;
+
+import java.io.IOException;
+import java.io.UncheckedIOException;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.util.concurrent.atomic.AtomicLong;
+
+import org.gradle.api.Action;
+import org.gradle.api.internal.tasks.testing.TestFramework;
+import org.gradle.api.internal.tasks.testing.WorkerTestClassProcessorFactory;
+import org.gradle.api.internal.tasks.testing.detection.TestFrameworkDetector;
+import org.gradle.api.tasks.testing.TestFrameworkOptions;
+import org.gradle.process.internal.JavaExecHandleBuilder;
+import org.gradle.process.internal.worker.WorkerProcessBuilder;
+
+/**
+ * Wraps a test framework to run each test worker in a separate working directory.
+ */
+public class RunInSubdirectoryTestFramework implements TestFramework {
+ private static final String GEMFIRE_PROPERTIES = "gemfire.properties";
+ private final AtomicLong workerId = new AtomicLong();
+ private final TestFramework delegate;
+
+ public RunInSubdirectoryTestFramework(TestFramework delegate) {
+ this.delegate = delegate;
+ }
+
+ @Override
+ public TestFrameworkDetector getDetector() {
+ return delegate.getDetector();
+ }
+
+ @Override
+ public TestFrameworkOptions getOptions() {
+ return delegate.getOptions();
+ }
+
+ @Override
+ public WorkerTestClassProcessorFactory getProcessorFactory() {
+ return delegate.getProcessorFactory();
+ }
+
+ /**
+ * Return an action that configures the test worker builder to run the test worker in a unique
+ * subdirectory of the task's working directory.
+ */
+ @Override
+ public Action<WorkerProcessBuilder> getWorkerConfigurationAction() {
+ return workerProcessBuilder -> {
+ delegate.getWorkerConfigurationAction().execute(workerProcessBuilder);
+ JavaExecHandleBuilder javaCommand = workerProcessBuilder.getJavaCommand();
Review comment:
I suspect it's not as simple as that, given that the Gradle client process needs to interact with the test worker process.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [geode] rhoughton-pivot commented on a change in pull request #5649: GEODE-8637: Give each test worker a unique working dir
Posted by GitBox <gi...@apache.org>.
rhoughton-pivot commented on a change in pull request #5649:
URL: https://github.com/apache/geode/pull/5649#discussion_r510315370
##########
File path: buildSrc/src/main/java/org/apache/geode/gradle/RunInSubdirectoryTestFramework.java
##########
@@ -0,0 +1,102 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more contributor license
+ * agreements. See the NOTICE file distributed with this work for additional information regarding
+ * copyright ownership. The ASF licenses this file to You under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the License. You may obtain a
+ * copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software distributed under the License
+ * is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express
+ * or implied. See the License for the specific language governing permissions and limitations under
+ * the License.
+ */
+
+package org.apache.geode.gradle;
+
+import static java.nio.file.StandardCopyOption.COPY_ATTRIBUTES;
+
+import java.io.IOException;
+import java.io.UncheckedIOException;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.util.concurrent.atomic.AtomicLong;
+
+import org.gradle.api.Action;
+import org.gradle.api.internal.tasks.testing.TestFramework;
+import org.gradle.api.internal.tasks.testing.WorkerTestClassProcessorFactory;
+import org.gradle.api.internal.tasks.testing.detection.TestFrameworkDetector;
+import org.gradle.api.tasks.testing.TestFrameworkOptions;
+import org.gradle.process.internal.JavaExecHandleBuilder;
+import org.gradle.process.internal.worker.WorkerProcessBuilder;
+
+/**
+ * Wraps a test framework to run each test worker in a separate working directory.
+ */
+public class RunInSubdirectoryTestFramework implements TestFramework {
+ private static final String GEMFIRE_PROPERTIES = "gemfire.properties";
+ private final AtomicLong workerId = new AtomicLong();
+ private final TestFramework delegate;
+
+ public RunInSubdirectoryTestFramework(TestFramework delegate) {
+ this.delegate = delegate;
+ }
+
+ @Override
+ public TestFrameworkDetector getDetector() {
+ return delegate.getDetector();
+ }
+
+ @Override
+ public TestFrameworkOptions getOptions() {
+ return delegate.getOptions();
+ }
+
+ @Override
+ public WorkerTestClassProcessorFactory getProcessorFactory() {
+ return delegate.getProcessorFactory();
+ }
+
+ /**
+ * Return an action that configures the test worker builder to run the test worker in a unique
+ * subdirectory of the task's working directory.
+ */
+ @Override
+ public Action<WorkerProcessBuilder> getWorkerConfigurationAction() {
+ return workerProcessBuilder -> {
+ delegate.getWorkerConfigurationAction().execute(workerProcessBuilder);
+ JavaExecHandleBuilder javaCommand = workerProcessBuilder.getJavaCommand();
+
+ Path taskWorkingDir = javaCommand.getWorkingDir().toPath();
+ String workerWorkingDirName = String.format("test-worker-%06d", workerId.incrementAndGet());
+ Path workerWorkingDir = taskWorkingDir.resolve(workerWorkingDirName);
+
+ createWorkingDir(workerWorkingDir);
+ copyGemFirePropertiesFile(taskWorkingDir, workerWorkingDir);
Review comment:
I am probably thinking of the gemfireVersion.properties :(
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [geode] jchen21 commented on a change in pull request #5649: GEODE-8637: Give each test worker a unique working dir
Posted by GitBox <gi...@apache.org>.
jchen21 commented on a change in pull request #5649:
URL: https://github.com/apache/geode/pull/5649#discussion_r510371381
##########
File path: buildSrc/src/main/java/org/apache/geode/gradle/RunInSubdirectoryTestFramework.java
##########
@@ -0,0 +1,102 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more contributor license
+ * agreements. See the NOTICE file distributed with this work for additional information regarding
+ * copyright ownership. The ASF licenses this file to You under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the License. You may obtain a
+ * copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software distributed under the License
+ * is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express
+ * or implied. See the License for the specific language governing permissions and limitations under
+ * the License.
+ */
+
+package org.apache.geode.gradle;
+
+import static java.nio.file.StandardCopyOption.COPY_ATTRIBUTES;
+
+import java.io.IOException;
+import java.io.UncheckedIOException;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.util.concurrent.atomic.AtomicLong;
+
+import org.gradle.api.Action;
+import org.gradle.api.internal.tasks.testing.TestFramework;
+import org.gradle.api.internal.tasks.testing.WorkerTestClassProcessorFactory;
+import org.gradle.api.internal.tasks.testing.detection.TestFrameworkDetector;
+import org.gradle.api.tasks.testing.TestFrameworkOptions;
+import org.gradle.process.internal.JavaExecHandleBuilder;
+import org.gradle.process.internal.worker.WorkerProcessBuilder;
+
+/**
+ * Wraps a test framework to run each test worker in a separate working directory.
+ */
+public class RunInSubdirectoryTestFramework implements TestFramework {
+ private static final String GEMFIRE_PROPERTIES = "gemfire.properties";
+ private final AtomicLong workerId = new AtomicLong();
+ private final TestFramework delegate;
+
+ public RunInSubdirectoryTestFramework(TestFramework delegate) {
+ this.delegate = delegate;
+ }
+
+ @Override
+ public TestFrameworkDetector getDetector() {
+ return delegate.getDetector();
+ }
+
+ @Override
+ public TestFrameworkOptions getOptions() {
+ return delegate.getOptions();
+ }
+
+ @Override
+ public WorkerTestClassProcessorFactory getProcessorFactory() {
+ return delegate.getProcessorFactory();
+ }
+
+ /**
+ * Return an action that configures the test worker builder to run the test worker in a unique
+ * subdirectory of the task's working directory.
+ */
+ @Override
+ public Action<WorkerProcessBuilder> getWorkerConfigurationAction() {
+ return workerProcessBuilder -> {
+ delegate.getWorkerConfigurationAction().execute(workerProcessBuilder);
+ JavaExecHandleBuilder javaCommand = workerProcessBuilder.getJavaCommand();
+
+ Path taskWorkingDir = javaCommand.getWorkingDir().toPath();
+ String workerWorkingDirName = String.format("test-worker-%06d", workerId.incrementAndGet());
+ Path workerWorkingDir = taskWorkingDir.resolve(workerWorkingDirName);
+
+ createWorkingDir(workerWorkingDir);
+ copyGemFirePropertiesFile(taskWorkingDir, workerWorkingDir);
+
+ javaCommand.setWorkingDir(workerWorkingDir);
+ };
+ }
+
+ private void copyGemFirePropertiesFile(Path taskWorkingDir, Path workerWorkingDir) {
+ Path taskPropertiesFile = taskWorkingDir.resolve(GEMFIRE_PROPERTIES);
+ if (!Files.exists(taskPropertiesFile)) {
+ return;
Review comment:
If `taskPropertiesFile` does not exist, will it affect `javaCommand.setWorkingDir(workerWorkingDir);` (line 78)? Or will it have some unexpected behavior when running tests?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [geode] upthewaterspout commented on a change in pull request #5649: GEODE-8637: Give each test worker a unique working dir
Posted by GitBox <gi...@apache.org>.
upthewaterspout commented on a change in pull request #5649:
URL: https://github.com/apache/geode/pull/5649#discussion_r509740082
##########
File path: buildSrc/src/main/java/org/apache/geode/gradle/RunInSubdirectoryTestFramework.java
##########
@@ -0,0 +1,102 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more contributor license
+ * agreements. See the NOTICE file distributed with this work for additional information regarding
+ * copyright ownership. The ASF licenses this file to You under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the License. You may obtain a
+ * copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software distributed under the License
+ * is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express
+ * or implied. See the License for the specific language governing permissions and limitations under
+ * the License.
+ */
+
+package org.apache.geode.gradle;
+
+import static java.nio.file.StandardCopyOption.COPY_ATTRIBUTES;
+
+import java.io.IOException;
+import java.io.UncheckedIOException;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.util.concurrent.atomic.AtomicLong;
+
+import org.gradle.api.Action;
+import org.gradle.api.internal.tasks.testing.TestFramework;
+import org.gradle.api.internal.tasks.testing.WorkerTestClassProcessorFactory;
+import org.gradle.api.internal.tasks.testing.detection.TestFrameworkDetector;
+import org.gradle.api.tasks.testing.TestFrameworkOptions;
+import org.gradle.process.internal.JavaExecHandleBuilder;
+import org.gradle.process.internal.worker.WorkerProcessBuilder;
+
+/**
+ * Wraps a test framework to run each test worker in a separate working directory.
+ */
+public class RunInSubdirectoryTestFramework implements TestFramework {
+ private static final String GEMFIRE_PROPERTIES = "gemfire.properties";
+ private final AtomicLong workerId = new AtomicLong();
+ private final TestFramework delegate;
+
+ public RunInSubdirectoryTestFramework(TestFramework delegate) {
+ this.delegate = delegate;
+ }
+
+ @Override
+ public TestFrameworkDetector getDetector() {
+ return delegate.getDetector();
+ }
+
+ @Override
+ public TestFrameworkOptions getOptions() {
+ return delegate.getOptions();
+ }
+
+ @Override
+ public WorkerTestClassProcessorFactory getProcessorFactory() {
+ return delegate.getProcessorFactory();
+ }
+
+ /**
+ * Return an action that configures the test worker builder to run the test worker in a unique
+ * subdirectory of the task's working directory.
+ */
+ @Override
+ public Action<WorkerProcessBuilder> getWorkerConfigurationAction() {
+ return workerProcessBuilder -> {
+ delegate.getWorkerConfigurationAction().execute(workerProcessBuilder);
+ JavaExecHandleBuilder javaCommand = workerProcessBuilder.getJavaCommand();
Review comment:
Yeah, I'm sure it's a bit more complicated. But maybe all of the gradle internal APIs that we would need to use are already in use in this PR?
Anyway, I wasn't being that serious with this comment, mostly just teasing you since I know you're trying to get rid of docker.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [geode] demery-pivotal commented on a change in pull request #5649: GEODE-8637: Give each test worker a unique working dir
Posted by GitBox <gi...@apache.org>.
demery-pivotal commented on a change in pull request #5649:
URL: https://github.com/apache/geode/pull/5649#discussion_r509745281
##########
File path: buildSrc/src/main/java/org/apache/geode/gradle/RunInSubdirectoryTestFramework.java
##########
@@ -0,0 +1,102 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more contributor license
+ * agreements. See the NOTICE file distributed with this work for additional information regarding
+ * copyright ownership. The ASF licenses this file to You under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the License. You may obtain a
+ * copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software distributed under the License
+ * is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express
+ * or implied. See the License for the specific language governing permissions and limitations under
+ * the License.
+ */
+
+package org.apache.geode.gradle;
+
+import static java.nio.file.StandardCopyOption.COPY_ATTRIBUTES;
+
+import java.io.IOException;
+import java.io.UncheckedIOException;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.util.concurrent.atomic.AtomicLong;
+
+import org.gradle.api.Action;
+import org.gradle.api.internal.tasks.testing.TestFramework;
+import org.gradle.api.internal.tasks.testing.WorkerTestClassProcessorFactory;
+import org.gradle.api.internal.tasks.testing.detection.TestFrameworkDetector;
+import org.gradle.api.tasks.testing.TestFrameworkOptions;
+import org.gradle.process.internal.JavaExecHandleBuilder;
+import org.gradle.process.internal.worker.WorkerProcessBuilder;
+
+/**
+ * Wraps a test framework to run each test worker in a separate working directory.
+ */
+public class RunInSubdirectoryTestFramework implements TestFramework {
+ private static final String GEMFIRE_PROPERTIES = "gemfire.properties";
+ private final AtomicLong workerId = new AtomicLong();
+ private final TestFramework delegate;
+
+ public RunInSubdirectoryTestFramework(TestFramework delegate) {
+ this.delegate = delegate;
+ }
+
+ @Override
+ public TestFrameworkDetector getDetector() {
+ return delegate.getDetector();
+ }
+
+ @Override
+ public TestFrameworkOptions getOptions() {
+ return delegate.getOptions();
+ }
+
+ @Override
+ public WorkerTestClassProcessorFactory getProcessorFactory() {
+ return delegate.getProcessorFactory();
+ }
+
+ /**
+ * Return an action that configures the test worker builder to run the test worker in a unique
+ * subdirectory of the task's working directory.
+ */
+ @Override
+ public Action<WorkerProcessBuilder> getWorkerConfigurationAction() {
+ return workerProcessBuilder -> {
+ delegate.getWorkerConfigurationAction().execute(workerProcessBuilder);
+ JavaExecHandleBuilder javaCommand = workerProcessBuilder.getJavaCommand();
Review comment:
I got the tease, but maybe you didn't know: We're not trying to get rid of docker per se. We're trying to get rid of dependence on an old, unmaintained dockerizer plugin that's holding us back. Now that I'm turning my attention back port conflicts, I find your not-serious idea unreasonably tempting! I'm not hopeful, but I will look into it.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [geode] demery-pivotal commented on a change in pull request #5649: GEODE-8637: Give each test worker a unique working dir
Posted by GitBox <gi...@apache.org>.
demery-pivotal commented on a change in pull request #5649:
URL: https://github.com/apache/geode/pull/5649#discussion_r509753151
##########
File path: buildSrc/src/main/java/org/apache/geode/gradle/RunInSubdirectoryTestFramework.java
##########
@@ -0,0 +1,102 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more contributor license
+ * agreements. See the NOTICE file distributed with this work for additional information regarding
+ * copyright ownership. The ASF licenses this file to You under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the License. You may obtain a
+ * copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software distributed under the License
+ * is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express
+ * or implied. See the License for the specific language governing permissions and limitations under
+ * the License.
+ */
+
+package org.apache.geode.gradle;
+
+import static java.nio.file.StandardCopyOption.COPY_ATTRIBUTES;
+
+import java.io.IOException;
+import java.io.UncheckedIOException;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.util.concurrent.atomic.AtomicLong;
+
+import org.gradle.api.Action;
+import org.gradle.api.internal.tasks.testing.TestFramework;
+import org.gradle.api.internal.tasks.testing.WorkerTestClassProcessorFactory;
+import org.gradle.api.internal.tasks.testing.detection.TestFrameworkDetector;
+import org.gradle.api.tasks.testing.TestFrameworkOptions;
+import org.gradle.process.internal.JavaExecHandleBuilder;
+import org.gradle.process.internal.worker.WorkerProcessBuilder;
+
+/**
+ * Wraps a test framework to run each test worker in a separate working directory.
+ */
+public class RunInSubdirectoryTestFramework implements TestFramework {
+ private static final String GEMFIRE_PROPERTIES = "gemfire.properties";
+ private final AtomicLong workerId = new AtomicLong();
+ private final TestFramework delegate;
+
+ public RunInSubdirectoryTestFramework(TestFramework delegate) {
+ this.delegate = delegate;
+ }
+
+ @Override
+ public TestFrameworkDetector getDetector() {
+ return delegate.getDetector();
+ }
+
+ @Override
+ public TestFrameworkOptions getOptions() {
+ return delegate.getOptions();
+ }
+
+ @Override
+ public WorkerTestClassProcessorFactory getProcessorFactory() {
+ return delegate.getProcessorFactory();
+ }
+
+ /**
+ * Return an action that configures the test worker builder to run the test worker in a unique
+ * subdirectory of the task's working directory.
+ */
+ @Override
+ public Action<WorkerProcessBuilder> getWorkerConfigurationAction() {
+ return workerProcessBuilder -> {
+ delegate.getWorkerConfigurationAction().execute(workerProcessBuilder);
+ JavaExecHandleBuilder javaCommand = workerProcessBuilder.getJavaCommand();
+
+ Path taskWorkingDir = javaCommand.getWorkingDir().toPath();
+ String workerWorkingDirName = String.format("test-worker-%06d", workerId.incrementAndGet());
+ Path workerWorkingDir = taskWorkingDir.resolve(workerWorkingDirName);
+
+ createWorkingDir(workerWorkingDir);
+ copyGemFirePropertiesFile(taskWorkingDir, workerWorkingDir);
Review comment:
Hmmm. If the point of that file is to prevent looking in other places… removing it might change behavior on developers' machines, if they have a ~/gemfire.properties file. And a run in CI would not notice the problem.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [geode] demery-pivotal merged pull request #5649: GEODE-8637: Give each test worker a unique working dir
Posted by GitBox <gi...@apache.org>.
demery-pivotal merged pull request #5649:
URL: https://github.com/apache/geode/pull/5649
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [geode] rhoughton-pivot commented on a change in pull request #5649: GEODE-8637: Give each test worker a unique working dir
Posted by GitBox <gi...@apache.org>.
rhoughton-pivot commented on a change in pull request #5649:
URL: https://github.com/apache/geode/pull/5649#discussion_r509721692
##########
File path: buildSrc/src/main/java/org/apache/geode/gradle/RunInSubdirectoryTestFramework.java
##########
@@ -0,0 +1,102 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more contributor license
+ * agreements. See the NOTICE file distributed with this work for additional information regarding
+ * copyright ownership. The ASF licenses this file to You under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the License. You may obtain a
+ * copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software distributed under the License
+ * is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express
+ * or implied. See the License for the specific language governing permissions and limitations under
+ * the License.
+ */
+
+package org.apache.geode.gradle;
+
+import static java.nio.file.StandardCopyOption.COPY_ATTRIBUTES;
+
+import java.io.IOException;
+import java.io.UncheckedIOException;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.util.concurrent.atomic.AtomicLong;
+
+import org.gradle.api.Action;
+import org.gradle.api.internal.tasks.testing.TestFramework;
+import org.gradle.api.internal.tasks.testing.WorkerTestClassProcessorFactory;
+import org.gradle.api.internal.tasks.testing.detection.TestFrameworkDetector;
+import org.gradle.api.tasks.testing.TestFrameworkOptions;
+import org.gradle.process.internal.JavaExecHandleBuilder;
+import org.gradle.process.internal.worker.WorkerProcessBuilder;
+
+/**
+ * Wraps a test framework to run each test worker in a separate working directory.
+ */
+public class RunInSubdirectoryTestFramework implements TestFramework {
+ private static final String GEMFIRE_PROPERTIES = "gemfire.properties";
+ private final AtomicLong workerId = new AtomicLong();
+ private final TestFramework delegate;
+
+ public RunInSubdirectoryTestFramework(TestFramework delegate) {
+ this.delegate = delegate;
+ }
+
+ @Override
+ public TestFrameworkDetector getDetector() {
+ return delegate.getDetector();
+ }
+
+ @Override
+ public TestFrameworkOptions getOptions() {
+ return delegate.getOptions();
+ }
+
+ @Override
+ public WorkerTestClassProcessorFactory getProcessorFactory() {
+ return delegate.getProcessorFactory();
+ }
+
+ /**
+ * Return an action that configures the test worker builder to run the test worker in a unique
+ * subdirectory of the task's working directory.
+ */
+ @Override
+ public Action<WorkerProcessBuilder> getWorkerConfigurationAction() {
+ return workerProcessBuilder -> {
+ delegate.getWorkerConfigurationAction().execute(workerProcessBuilder);
+ JavaExecHandleBuilder javaCommand = workerProcessBuilder.getJavaCommand();
+
+ Path taskWorkingDir = javaCommand.getWorkingDir().toPath();
+ String workerWorkingDirName = String.format("test-worker-%06d", workerId.incrementAndGet());
+ Path workerWorkingDir = taskWorkingDir.resolve(workerWorkingDirName);
+
+ createWorkingDir(workerWorkingDir);
+ copyGemFirePropertiesFile(taskWorkingDir, workerWorkingDir);
Review comment:
The gemfire.properties files, in all its incarnations, has been a thorn to me, because the writing of it causes a repack of geode-core.jar, which triggers all kinds of downstream effects.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [geode] demery-pivotal commented on a change in pull request #5649: GEODE-8637: Give each test worker a unique working dir
Posted by GitBox <gi...@apache.org>.
demery-pivotal commented on a change in pull request #5649:
URL: https://github.com/apache/geode/pull/5649#discussion_r510390095
##########
File path: buildSrc/src/main/java/org/apache/geode/gradle/RunInSubdirectoryTestFramework.java
##########
@@ -0,0 +1,102 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more contributor license
+ * agreements. See the NOTICE file distributed with this work for additional information regarding
+ * copyright ownership. The ASF licenses this file to You under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the License. You may obtain a
+ * copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software distributed under the License
+ * is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express
+ * or implied. See the License for the specific language governing permissions and limitations under
+ * the License.
+ */
+
+package org.apache.geode.gradle;
+
+import static java.nio.file.StandardCopyOption.COPY_ATTRIBUTES;
+
+import java.io.IOException;
+import java.io.UncheckedIOException;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.util.concurrent.atomic.AtomicLong;
+
+import org.gradle.api.Action;
+import org.gradle.api.internal.tasks.testing.TestFramework;
+import org.gradle.api.internal.tasks.testing.WorkerTestClassProcessorFactory;
+import org.gradle.api.internal.tasks.testing.detection.TestFrameworkDetector;
+import org.gradle.api.tasks.testing.TestFrameworkOptions;
+import org.gradle.process.internal.JavaExecHandleBuilder;
+import org.gradle.process.internal.worker.WorkerProcessBuilder;
+
+/**
+ * Wraps a test framework to run each test worker in a separate working directory.
+ */
+public class RunInSubdirectoryTestFramework implements TestFramework {
+ private static final String GEMFIRE_PROPERTIES = "gemfire.properties";
+ private final AtomicLong workerId = new AtomicLong();
+ private final TestFramework delegate;
+
+ public RunInSubdirectoryTestFramework(TestFramework delegate) {
+ this.delegate = delegate;
+ }
+
+ @Override
+ public TestFrameworkDetector getDetector() {
+ return delegate.getDetector();
+ }
+
+ @Override
+ public TestFrameworkOptions getOptions() {
+ return delegate.getOptions();
+ }
+
+ @Override
+ public WorkerTestClassProcessorFactory getProcessorFactory() {
+ return delegate.getProcessorFactory();
+ }
+
+ /**
+ * Return an action that configures the test worker builder to run the test worker in a unique
+ * subdirectory of the task's working directory.
+ */
+ @Override
+ public Action<WorkerProcessBuilder> getWorkerConfigurationAction() {
+ return workerProcessBuilder -> {
+ delegate.getWorkerConfigurationAction().execute(workerProcessBuilder);
+ JavaExecHandleBuilder javaCommand = workerProcessBuilder.getJavaCommand();
+
+ Path taskWorkingDir = javaCommand.getWorkingDir().toPath();
+ String workerWorkingDirName = String.format("test-worker-%06d", workerId.incrementAndGet());
+ Path workerWorkingDir = taskWorkingDir.resolve(workerWorkingDirName);
+
+ createWorkingDir(workerWorkingDir);
+ copyGemFirePropertiesFile(taskWorkingDir, workerWorkingDir);
+
+ javaCommand.setWorkingDir(workerWorkingDir);
+ };
+ }
+
+ private void copyGemFirePropertiesFile(Path taskWorkingDir, Path workerWorkingDir) {
+ Path taskPropertiesFile = taskWorkingDir.resolve(GEMFIRE_PROPERTIES);
+ if (!Files.exists(taskPropertiesFile)) {
+ return;
Review comment:
If the file is missing, that doesn't indicate a problem. It's deliberate. Our Gradle build puts that file there for some test tasks (e.g. distributedTest) but not for others (e.g. repeatDistributedTest). I don't know why our Gradle build writes the file for some tasks and not others, but that's the current behavior.
The if statement accounts for Gradle writing the file for some tasks and not for others.
The effect is that if the file was in the test JVM's common working directory _before_ this PR, it will be in the JVM's unique working directory _after._ And if the file was absent from the test JVM's working directory before the PR, it will be absent after.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [geode] demery-pivotal commented on a change in pull request #5649: GEODE-8637: Give each test worker a unique working dir
Posted by GitBox <gi...@apache.org>.
demery-pivotal commented on a change in pull request #5649:
URL: https://github.com/apache/geode/pull/5649#discussion_r509710565
##########
File path: buildSrc/src/main/java/org/apache/geode/gradle/RunInSubdirectoryTestFramework.java
##########
@@ -0,0 +1,102 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more contributor license
+ * agreements. See the NOTICE file distributed with this work for additional information regarding
+ * copyright ownership. The ASF licenses this file to You under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the License. You may obtain a
+ * copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software distributed under the License
+ * is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express
+ * or implied. See the License for the specific language governing permissions and limitations under
+ * the License.
+ */
+
+package org.apache.geode.gradle;
+
+import static java.nio.file.StandardCopyOption.COPY_ATTRIBUTES;
+
+import java.io.IOException;
+import java.io.UncheckedIOException;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.util.concurrent.atomic.AtomicLong;
+
+import org.gradle.api.Action;
+import org.gradle.api.internal.tasks.testing.TestFramework;
+import org.gradle.api.internal.tasks.testing.WorkerTestClassProcessorFactory;
+import org.gradle.api.internal.tasks.testing.detection.TestFrameworkDetector;
+import org.gradle.api.tasks.testing.TestFrameworkOptions;
+import org.gradle.process.internal.JavaExecHandleBuilder;
+import org.gradle.process.internal.worker.WorkerProcessBuilder;
+
+/**
+ * Wraps a test framework to run each test worker in a separate working directory.
+ */
+public class RunInSubdirectoryTestFramework implements TestFramework {
+ private static final String GEMFIRE_PROPERTIES = "gemfire.properties";
+ private final AtomicLong workerId = new AtomicLong();
+ private final TestFramework delegate;
+
+ public RunInSubdirectoryTestFramework(TestFramework delegate) {
+ this.delegate = delegate;
+ }
+
+ @Override
+ public TestFrameworkDetector getDetector() {
+ return delegate.getDetector();
+ }
+
+ @Override
+ public TestFrameworkOptions getOptions() {
+ return delegate.getOptions();
+ }
+
+ @Override
+ public WorkerTestClassProcessorFactory getProcessorFactory() {
+ return delegate.getProcessorFactory();
+ }
+
+ /**
+ * Return an action that configures the test worker builder to run the test worker in a unique
+ * subdirectory of the task's working directory.
+ */
+ @Override
+ public Action<WorkerProcessBuilder> getWorkerConfigurationAction() {
+ return workerProcessBuilder -> {
+ delegate.getWorkerConfigurationAction().execute(workerProcessBuilder);
+ JavaExecHandleBuilder javaCommand = workerProcessBuilder.getJavaCommand();
+
+ Path taskWorkingDir = javaCommand.getWorkingDir().toPath();
+ String workerWorkingDirName = String.format("test-worker-%06d", workerId.incrementAndGet());
+ Path workerWorkingDir = taskWorkingDir.resolve(workerWorkingDirName);
+
+ createWorkingDir(workerWorkingDir);
+ copyGemFirePropertiesFile(taskWorkingDir, workerWorkingDir);
Review comment:
Just today I noticed that the non-repeat forms of these tasks don't write as properties file. If it's not needed, it would probably be better to delete the code that writes it.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [geode] upthewaterspout commented on a change in pull request #5649: GEODE-8637: Give each test worker a unique working dir
Posted by GitBox <gi...@apache.org>.
upthewaterspout commented on a change in pull request #5649:
URL: https://github.com/apache/geode/pull/5649#discussion_r509701747
##########
File path: buildSrc/src/main/java/org/apache/geode/gradle/RunInSubdirectoryTestFramework.java
##########
@@ -0,0 +1,102 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more contributor license
+ * agreements. See the NOTICE file distributed with this work for additional information regarding
+ * copyright ownership. The ASF licenses this file to You under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the License. You may obtain a
+ * copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software distributed under the License
+ * is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express
+ * or implied. See the License for the specific language governing permissions and limitations under
+ * the License.
+ */
+
+package org.apache.geode.gradle;
+
+import static java.nio.file.StandardCopyOption.COPY_ATTRIBUTES;
+
+import java.io.IOException;
+import java.io.UncheckedIOException;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.util.concurrent.atomic.AtomicLong;
+
+import org.gradle.api.Action;
+import org.gradle.api.internal.tasks.testing.TestFramework;
+import org.gradle.api.internal.tasks.testing.WorkerTestClassProcessorFactory;
+import org.gradle.api.internal.tasks.testing.detection.TestFrameworkDetector;
+import org.gradle.api.tasks.testing.TestFrameworkOptions;
+import org.gradle.process.internal.JavaExecHandleBuilder;
+import org.gradle.process.internal.worker.WorkerProcessBuilder;
+
+/**
+ * Wraps a test framework to run each test worker in a separate working directory.
+ */
+public class RunInSubdirectoryTestFramework implements TestFramework {
+ private static final String GEMFIRE_PROPERTIES = "gemfire.properties";
+ private final AtomicLong workerId = new AtomicLong();
+ private final TestFramework delegate;
+
+ public RunInSubdirectoryTestFramework(TestFramework delegate) {
+ this.delegate = delegate;
+ }
+
+ @Override
+ public TestFrameworkDetector getDetector() {
+ return delegate.getDetector();
+ }
+
+ @Override
+ public TestFrameworkOptions getOptions() {
+ return delegate.getOptions();
+ }
+
+ @Override
+ public WorkerTestClassProcessorFactory getProcessorFactory() {
+ return delegate.getProcessorFactory();
+ }
+
+ /**
+ * Return an action that configures the test worker builder to run the test worker in a unique
+ * subdirectory of the task's working directory.
+ */
+ @Override
+ public Action<WorkerProcessBuilder> getWorkerConfigurationAction() {
+ return workerProcessBuilder -> {
+ delegate.getWorkerConfigurationAction().execute(workerProcessBuilder);
+ JavaExecHandleBuilder javaCommand = workerProcessBuilder.getJavaCommand();
Review comment:
Huh, I wonder if you could do something like javaCommand.setExecutable("docker") ... to run the workers in a docker container ;)
##########
File path: buildSrc/src/main/java/org/apache/geode/gradle/RunInSubdirectoryTestFramework.java
##########
@@ -0,0 +1,102 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more contributor license
+ * agreements. See the NOTICE file distributed with this work for additional information regarding
+ * copyright ownership. The ASF licenses this file to You under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the License. You may obtain a
+ * copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software distributed under the License
+ * is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express
+ * or implied. See the License for the specific language governing permissions and limitations under
+ * the License.
+ */
+
+package org.apache.geode.gradle;
+
+import static java.nio.file.StandardCopyOption.COPY_ATTRIBUTES;
+
+import java.io.IOException;
+import java.io.UncheckedIOException;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.util.concurrent.atomic.AtomicLong;
+
+import org.gradle.api.Action;
+import org.gradle.api.internal.tasks.testing.TestFramework;
+import org.gradle.api.internal.tasks.testing.WorkerTestClassProcessorFactory;
+import org.gradle.api.internal.tasks.testing.detection.TestFrameworkDetector;
+import org.gradle.api.tasks.testing.TestFrameworkOptions;
+import org.gradle.process.internal.JavaExecHandleBuilder;
+import org.gradle.process.internal.worker.WorkerProcessBuilder;
+
+/**
+ * Wraps a test framework to run each test worker in a separate working directory.
+ */
+public class RunInSubdirectoryTestFramework implements TestFramework {
+ private static final String GEMFIRE_PROPERTIES = "gemfire.properties";
+ private final AtomicLong workerId = new AtomicLong();
+ private final TestFramework delegate;
+
+ public RunInSubdirectoryTestFramework(TestFramework delegate) {
+ this.delegate = delegate;
+ }
+
+ @Override
+ public TestFrameworkDetector getDetector() {
+ return delegate.getDetector();
+ }
+
+ @Override
+ public TestFrameworkOptions getOptions() {
+ return delegate.getOptions();
+ }
+
+ @Override
+ public WorkerTestClassProcessorFactory getProcessorFactory() {
+ return delegate.getProcessorFactory();
+ }
+
+ /**
+ * Return an action that configures the test worker builder to run the test worker in a unique
+ * subdirectory of the task's working directory.
+ */
+ @Override
+ public Action<WorkerProcessBuilder> getWorkerConfigurationAction() {
+ return workerProcessBuilder -> {
+ delegate.getWorkerConfigurationAction().execute(workerProcessBuilder);
+ JavaExecHandleBuilder javaCommand = workerProcessBuilder.getJavaCommand();
+
+ Path taskWorkingDir = javaCommand.getWorkingDir().toPath();
+ String workerWorkingDirName = String.format("test-worker-%06d", workerId.incrementAndGet());
+ Path workerWorkingDir = taskWorkingDir.resolve(workerWorkingDirName);
+
+ createWorkingDir(workerWorkingDir);
+ copyGemFirePropertiesFile(taskWorkingDir, workerWorkingDir);
Review comment:
I wonder if we need/want this gemfire.properties file anymore? As far as I'm concerned tests shouldn't be running with a gemfire.properties file lying around anyway?
I see we have TestPropertiesWriter that creates a properties file. But all it has is log-level=config, which is the default anyway...
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [geode] demery-pivotal commented on a change in pull request #5649: GEODE-8637: Give each test worker a unique working dir
Posted by GitBox <gi...@apache.org>.
demery-pivotal commented on a change in pull request #5649:
URL: https://github.com/apache/geode/pull/5649#discussion_r509711684
##########
File path: buildSrc/src/main/java/org/apache/geode/gradle/RunInSubdirectoryTestFramework.java
##########
@@ -0,0 +1,102 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more contributor license
+ * agreements. See the NOTICE file distributed with this work for additional information regarding
+ * copyright ownership. The ASF licenses this file to You under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the License. You may obtain a
+ * copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software distributed under the License
+ * is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express
+ * or implied. See the License for the specific language governing permissions and limitations under
+ * the License.
+ */
+
+package org.apache.geode.gradle;
+
+import static java.nio.file.StandardCopyOption.COPY_ATTRIBUTES;
+
+import java.io.IOException;
+import java.io.UncheckedIOException;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.util.concurrent.atomic.AtomicLong;
+
+import org.gradle.api.Action;
+import org.gradle.api.internal.tasks.testing.TestFramework;
+import org.gradle.api.internal.tasks.testing.WorkerTestClassProcessorFactory;
+import org.gradle.api.internal.tasks.testing.detection.TestFrameworkDetector;
+import org.gradle.api.tasks.testing.TestFrameworkOptions;
+import org.gradle.process.internal.JavaExecHandleBuilder;
+import org.gradle.process.internal.worker.WorkerProcessBuilder;
+
+/**
+ * Wraps a test framework to run each test worker in a separate working directory.
+ */
+public class RunInSubdirectoryTestFramework implements TestFramework {
+ private static final String GEMFIRE_PROPERTIES = "gemfire.properties";
+ private final AtomicLong workerId = new AtomicLong();
+ private final TestFramework delegate;
+
+ public RunInSubdirectoryTestFramework(TestFramework delegate) {
+ this.delegate = delegate;
+ }
+
+ @Override
+ public TestFrameworkDetector getDetector() {
+ return delegate.getDetector();
+ }
+
+ @Override
+ public TestFrameworkOptions getOptions() {
+ return delegate.getOptions();
+ }
+
+ @Override
+ public WorkerTestClassProcessorFactory getProcessorFactory() {
+ return delegate.getProcessorFactory();
+ }
+
+ /**
+ * Return an action that configures the test worker builder to run the test worker in a unique
+ * subdirectory of the task's working directory.
+ */
+ @Override
+ public Action<WorkerProcessBuilder> getWorkerConfigurationAction() {
+ return workerProcessBuilder -> {
+ delegate.getWorkerConfigurationAction().execute(workerProcessBuilder);
+ JavaExecHandleBuilder javaCommand = workerProcessBuilder.getJavaCommand();
Review comment:
That's a really interesting idea!
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [geode] upthewaterspout commented on a change in pull request #5649: GEODE-8637: Give each test worker a unique working dir
Posted by GitBox <gi...@apache.org>.
upthewaterspout commented on a change in pull request #5649:
URL: https://github.com/apache/geode/pull/5649#discussion_r509741370
##########
File path: buildSrc/src/main/java/org/apache/geode/gradle/RunInSubdirectoryTestFramework.java
##########
@@ -0,0 +1,102 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more contributor license
+ * agreements. See the NOTICE file distributed with this work for additional information regarding
+ * copyright ownership. The ASF licenses this file to You under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the License. You may obtain a
+ * copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software distributed under the License
+ * is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express
+ * or implied. See the License for the specific language governing permissions and limitations under
+ * the License.
+ */
+
+package org.apache.geode.gradle;
+
+import static java.nio.file.StandardCopyOption.COPY_ATTRIBUTES;
+
+import java.io.IOException;
+import java.io.UncheckedIOException;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.util.concurrent.atomic.AtomicLong;
+
+import org.gradle.api.Action;
+import org.gradle.api.internal.tasks.testing.TestFramework;
+import org.gradle.api.internal.tasks.testing.WorkerTestClassProcessorFactory;
+import org.gradle.api.internal.tasks.testing.detection.TestFrameworkDetector;
+import org.gradle.api.tasks.testing.TestFrameworkOptions;
+import org.gradle.process.internal.JavaExecHandleBuilder;
+import org.gradle.process.internal.worker.WorkerProcessBuilder;
+
+/**
+ * Wraps a test framework to run each test worker in a separate working directory.
+ */
+public class RunInSubdirectoryTestFramework implements TestFramework {
+ private static final String GEMFIRE_PROPERTIES = "gemfire.properties";
+ private final AtomicLong workerId = new AtomicLong();
+ private final TestFramework delegate;
+
+ public RunInSubdirectoryTestFramework(TestFramework delegate) {
+ this.delegate = delegate;
+ }
+
+ @Override
+ public TestFrameworkDetector getDetector() {
+ return delegate.getDetector();
+ }
+
+ @Override
+ public TestFrameworkOptions getOptions() {
+ return delegate.getOptions();
+ }
+
+ @Override
+ public WorkerTestClassProcessorFactory getProcessorFactory() {
+ return delegate.getProcessorFactory();
+ }
+
+ /**
+ * Return an action that configures the test worker builder to run the test worker in a unique
+ * subdirectory of the task's working directory.
+ */
+ @Override
+ public Action<WorkerProcessBuilder> getWorkerConfigurationAction() {
+ return workerProcessBuilder -> {
+ delegate.getWorkerConfigurationAction().execute(workerProcessBuilder);
+ JavaExecHandleBuilder javaCommand = workerProcessBuilder.getJavaCommand();
+
+ Path taskWorkingDir = javaCommand.getWorkingDir().toPath();
+ String workerWorkingDirName = String.format("test-worker-%06d", workerId.incrementAndGet());
+ Path workerWorkingDir = taskWorkingDir.resolve(workerWorkingDirName);
+
+ createWorkingDir(workerWorkingDir);
+ copyGemFirePropertiesFile(taskWorkingDir, workerWorkingDir);
Review comment:
I don't think it is? The one thing maybe it might do is prevent us from looking for a gemfire.properties file in other places (the classpath?, the user's homedir?). Could be worth a run if we can get rid of some code.
BTW, @rhoughton-pivot - I think maybe you are talking about writing the *default* properties file? That's different code I believe. But yeah, still seems like in that case maybe we could just check in a file or something.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org