Skip to content
This repository has been archived by the owner on Jan 19, 2022. It is now read-only.

pubsub emulator rule moved to a separate project #2352

Closed
wants to merge 3 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@
<module>spring-cloud-gcp-bigquery</module>
<module>spring-cloud-gcp-security-firebase</module>
<module>spring-cloud-gcp-secretmanager</module>
<module>spring-cloud-gcp-test</module>
</modules>

<properties>
Expand Down
6 changes: 6 additions & 0 deletions spring-cloud-gcp-dependencies/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -191,6 +191,12 @@
<version>${project.version}</version>
</dependency>

<dependency>
<groupId>org.springframework.cloud</groupId>
<artifactId>spring-cloud-gcp-test</artifactId>
<version>${project.version}</version>
</dependency>

<!-- spring-cloud-gcp-starter-sql -->
<dependency>
<groupId>com.google.cloud.sql</groupId>
Expand Down
6 changes: 6 additions & 0 deletions spring-cloud-gcp-pubsub-stream-binder/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -42,5 +42,11 @@
<version>3.1.0.BUILD-SNAPSHOT</version>
<scope>test</scope>
</dependency>
<dependency>
<groupId>org.springframework.cloud</groupId>
<artifactId>spring-cloud-gcp-test</artifactId>
<version>${project.version}</version>
<scope>test</scope>
</dependency>
</dependencies>
</project>
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@

import org.springframework.cloud.gcp.stream.binder.pubsub.properties.PubSubConsumerProperties;
import org.springframework.cloud.gcp.stream.binder.pubsub.properties.PubSubProducerProperties;
import org.springframework.cloud.gcp.test.PubSubEmulator;
import org.springframework.cloud.stream.binder.AbstractBinderTests;
import org.springframework.cloud.stream.binder.ExtendedConsumerProperties;
import org.springframework.cloud.stream.binder.ExtendedProducerProperties;
Expand Down
29 changes: 29 additions & 0 deletions spring-cloud-gcp-test/pom.xml
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
<?xml version="1.0" encoding="UTF-8"?>
<project xmlns="http://maven.apache.org/POM/4.0.0"
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">
<modelVersion>4.0.0</modelVersion>
<parent>
<artifactId>spring-cloud-gcp</artifactId>
<groupId>org.springframework.cloud</groupId>
<version>1.2.3.BUILD-SNAPSHOT</version>
</parent>


<artifactId>spring-cloud-gcp-test</artifactId>
<name>Spring Cloud GCP Test Support</name>
<description>Provides tools for testing Spring Cloud GCP projects</description>
<dependencies>
<dependency>
<groupId>junit</groupId>
<artifactId>junit</artifactId>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

test scope?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not in this case -- the test project itself is meant to be imported as test scope, but the rule extends a JUnit class, so it needs to be compile, not test scope.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if we should consider making these utilities independent of JUnit. People use different test frameworks.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

make it generic so people can reuse it sounds good. Can you take into account that may some people will be able to contribute creating rules for junit4 or extension for junit5 specifically? Those can be issues with ideal for contribution or help wanted. WDYT?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That a good idea! We should do that.

</dependency>

<dependency>
<groupId>org.springframework</groupId>
<artifactId>spring-jcl</artifactId>
<scope>compile</scope>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Compile scope is the default, you don't have to specify it.

</dependency>
</dependencies>

</project>
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
* limitations under the License.
*/

package org.springframework.cloud.gcp.stream.binder.pubsub;
package org.springframework.cloud.gcp.test;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
package org.springframework.cloud.gcp.test;
package org.springframework.cloud.gcp.test.pubsub;


import java.io.BufferedReader;
import java.io.IOException;
Expand All @@ -33,11 +33,10 @@

import org.apache.commons.logging.Log;
import org.apache.commons.logging.LogFactory;
import org.junit.Assert;
import org.junit.Assume;
import org.junit.rules.ExternalResource;

import static org.junit.Assert.fail;
import static org.junit.Assume.assumeTrue;

/**
* Rule for instantiating and tearing down a Pub/Sub emulator instance.
*
Expand Down Expand Up @@ -87,7 +86,7 @@ public PubSubEmulator() {
@Override
protected void before() throws IOException, InterruptedException {

assumeTrue("PubSubEmulator rule disabled. Please enable with -Dit.pubsub-emulator.", this.enableTests);
Assume.assumeTrue("PubSubEmulator rule disabled. Please enable with -Dit.pubsub-emulator.", this.enableTests);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think that enabling it with -Dit.pubsub-emulator makes sense when you're offering it as a generic test utility. That flag was specific for our project.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's an interesting question then to test whether the rule initialization or a test's @BeforeClass will run first. We would not want to do emulator startup if the tests themselves are disabled.

If there is possibility of the rule running first, perhaps we can just document this flag well?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can also try not to implement JUnit test rule.
That's what we ended up doing for Spring Cloud Function here.


startEmulator();
determineHostPort();
Expand Down Expand Up @@ -178,7 +177,7 @@ private void startEmulator() throws IOException, InterruptedException {
.start();
}
catch (IOException ex) {
fail("Gcloud not found; leaving host/port uninitialized.");
Assert.fail("Gcloud not found; leaving host/port uninitialized.");
}

if (configPresent) {
Expand All @@ -188,7 +187,6 @@ private void startEmulator() throws IOException, InterruptedException {
else {
createConfig();
}

}

/**
Expand All @@ -214,12 +212,12 @@ private void determineHostPort() throws IOException, InterruptedException {
* to fail the test.
*/
private void createConfig() throws InterruptedException {
int attempts = 10;
int attempts = 100;
while (!Files.exists(EMULATOR_CONFIG_PATH) && --attempts >= 0) {
Thread.sleep(1000);
}
if (attempts < 0) {
fail(
Assert.fail(
"Emulator could not be configured due to missing env.yaml. Are PubSub and beta tools installed?");
}
}
Expand All @@ -234,7 +232,7 @@ private void createConfig() throws InterruptedException {
private void updateConfig(WatchService watchService) throws InterruptedException {
int attempts = 10;
while (--attempts >= 0) {
WatchKey key = watchService.poll(100, TimeUnit.MILLISECONDS);
WatchKey key = watchService.poll(1, TimeUnit.SECONDS);

if (key != null) {
Optional<Path> configFilePath = key.pollEvents().stream()
Expand All @@ -247,7 +245,7 @@ private void updateConfig(WatchService watchService) throws InterruptedException
}
}

fail("Configuration file update could not be detected");
Assert.fail("Configuration file update could not be detected");
}

/**
Expand Down