You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@camel.apache.org by GitBox <gi...@apache.org> on 2020/11/04 08:02:18 UTC
[GitHub] [camel-quarkus] jamesnetherton opened a new pull request #1988: WireMockTestResourceLifecycleManager improvements
jamesnetherton opened a new pull request #1988:
URL: https://github.com/apache/camel-quarkus/pull/1988
----------------------------------------------------------------
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] [camel-quarkus] jamesnetherton commented on a change in pull request #1988: WireMockTestResourceLifecycleManager improvements
Posted by GitBox <gi...@apache.org>.
jamesnetherton commented on a change in pull request #1988:
URL: https://github.com/apache/camel-quarkus/pull/1988#discussion_r517192117
##########
File path: integration-tests-support/wiremock/src/main/java/org/apache/camel/quarkus/test/wiremock/WireMockTestResourceLifecycleManager.java
##########
@@ -78,30 +82,35 @@
*/
@Override
public void stop() {
- if (server != null) {
- LOG.info("Stopping WireMockServer");
- if (server.getRecordingStatus().getStatus().equals(RecordingStatus.Recording)) {
- LOG.info("Stopping recording");
- SnapshotRecordResult recordResult = server.stopRecording();
-
- List<StubMapping> stubMappings = recordResult.getStubMappings();
- if (isDeleteRecordedMappingsOnError()) {
- for (StubMapping mapping : stubMappings) {
- int status = mapping.getResponse().getStatus();
- if (status >= 300 && mapping.shouldBePersisted()) {
- try {
- String fileName = mapping.getName() + "-" + mapping.getId() + ".json";
- Path mappingFilePath = Paths.get("./src/test/resources/mappings/", fileName);
- Files.deleteIfExists(mappingFilePath);
- LOG.infof("Deleted mapping file %s as status code was %d", fileName, status);
- } catch (IOException e) {
- LOG.errorf("Failed to delete mapping file %s", e, mapping.getName());
+ WireMockServer server = getServer();
+ try {
+ if (server != null) {
+ LOG.info("Stopping WireMockServer");
+ if (server.getRecordingStatus().getStatus().equals(RecordingStatus.Recording)) {
+ LOG.info("Stopping recording");
+ SnapshotRecordResult recordResult = server.stopRecording();
+
+ List<StubMapping> stubMappings = recordResult.getStubMappings();
+ if (isDeleteRecordedMappingsOnError()) {
+ for (StubMapping mapping : stubMappings) {
+ int status = mapping.getResponse().getStatus();
+ if (status >= 300 && mapping.shouldBePersisted()) {
+ try {
+ String fileName = mapping.getName() + "-" + mapping.getId() + ".json";
+ Path mappingFilePath = Paths.get("./src/test/resources/mappings/", fileName);
+ Files.deleteIfExists(mappingFilePath);
+ LOG.infof("Deleted mapping file %s as status code was %d", fileName, status);
+ } catch (IOException e) {
+ LOG.errorf("Failed to delete mapping file %s", e, mapping.getName());
+ }
}
}
}
}
+ server.stop();
}
- server.stop();
+ } finally {
+ servers.remove(getServerKey());
Review comment:
If an integration test has multiple test classes annotated with `@TestResource(Foo.class)`. Then `start()` / `stop()` is only invoked once. I.e when the first test starts and the last test completes.
----------------------------------------------------------------
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] [camel-quarkus] ppalaga commented on a change in pull request #1988: WireMockTestResourceLifecycleManager improvements
Posted by GitBox <gi...@apache.org>.
ppalaga commented on a change in pull request #1988:
URL: https://github.com/apache/camel-quarkus/pull/1988#discussion_r517204637
##########
File path: integration-tests-support/wiremock/src/main/java/org/apache/camel/quarkus/test/wiremock/WireMockTestResourceLifecycleManager.java
##########
@@ -78,30 +82,35 @@
*/
@Override
public void stop() {
- if (server != null) {
- LOG.info("Stopping WireMockServer");
- if (server.getRecordingStatus().getStatus().equals(RecordingStatus.Recording)) {
- LOG.info("Stopping recording");
- SnapshotRecordResult recordResult = server.stopRecording();
-
- List<StubMapping> stubMappings = recordResult.getStubMappings();
- if (isDeleteRecordedMappingsOnError()) {
- for (StubMapping mapping : stubMappings) {
- int status = mapping.getResponse().getStatus();
- if (status >= 300 && mapping.shouldBePersisted()) {
- try {
- String fileName = mapping.getName() + "-" + mapping.getId() + ".json";
- Path mappingFilePath = Paths.get("./src/test/resources/mappings/", fileName);
- Files.deleteIfExists(mappingFilePath);
- LOG.infof("Deleted mapping file %s as status code was %d", fileName, status);
- } catch (IOException e) {
- LOG.errorf("Failed to delete mapping file %s", e, mapping.getName());
+ WireMockServer server = getServer();
+ try {
+ if (server != null) {
+ LOG.info("Stopping WireMockServer");
+ if (server.getRecordingStatus().getStatus().equals(RecordingStatus.Recording)) {
+ LOG.info("Stopping recording");
+ SnapshotRecordResult recordResult = server.stopRecording();
+
+ List<StubMapping> stubMappings = recordResult.getStubMappings();
+ if (isDeleteRecordedMappingsOnError()) {
+ for (StubMapping mapping : stubMappings) {
+ int status = mapping.getResponse().getStatus();
+ if (status >= 300 && mapping.shouldBePersisted()) {
+ try {
+ String fileName = mapping.getName() + "-" + mapping.getId() + ".json";
+ Path mappingFilePath = Paths.get("./src/test/resources/mappings/", fileName);
+ Files.deleteIfExists(mappingFilePath);
+ LOG.infof("Deleted mapping file %s as status code was %d", fileName, status);
+ } catch (IOException e) {
+ LOG.errorf("Failed to delete mapping file %s", e, mapping.getName());
+ }
}
}
}
}
+ server.stop();
}
- server.stop();
+ } finally {
+ servers.remove(getServerKey());
Review comment:
I see. Is the TestResourceLifecycleManager instance on which start() and stop() are called different?
A comment somewhere would be nice so that future readers do not need to wonder.
----------------------------------------------------------------
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] [camel-quarkus] jamesnetherton closed pull request #1988: WireMockTestResourceLifecycleManager improvements
Posted by GitBox <gi...@apache.org>.
jamesnetherton closed pull request #1988:
URL: https://github.com/apache/camel-quarkus/pull/1988
----------------------------------------------------------------
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] [camel-quarkus] jamesnetherton commented on a change in pull request #1988: WireMockTestResourceLifecycleManager improvements
Posted by GitBox <gi...@apache.org>.
jamesnetherton commented on a change in pull request #1988:
URL: https://github.com/apache/camel-quarkus/pull/1988#discussion_r517218920
##########
File path: integration-tests-support/wiremock/src/main/java/org/apache/camel/quarkus/test/wiremock/WireMockTestResourceLifecycleManager.java
##########
@@ -78,30 +82,35 @@
*/
@Override
public void stop() {
- if (server != null) {
- LOG.info("Stopping WireMockServer");
- if (server.getRecordingStatus().getStatus().equals(RecordingStatus.Recording)) {
- LOG.info("Stopping recording");
- SnapshotRecordResult recordResult = server.stopRecording();
-
- List<StubMapping> stubMappings = recordResult.getStubMappings();
- if (isDeleteRecordedMappingsOnError()) {
- for (StubMapping mapping : stubMappings) {
- int status = mapping.getResponse().getStatus();
- if (status >= 300 && mapping.shouldBePersisted()) {
- try {
- String fileName = mapping.getName() + "-" + mapping.getId() + ".json";
- Path mappingFilePath = Paths.get("./src/test/resources/mappings/", fileName);
- Files.deleteIfExists(mappingFilePath);
- LOG.infof("Deleted mapping file %s as status code was %d", fileName, status);
- } catch (IOException e) {
- LOG.errorf("Failed to delete mapping file %s", e, mapping.getName());
+ WireMockServer server = getServer();
+ try {
+ if (server != null) {
+ LOG.info("Stopping WireMockServer");
+ if (server.getRecordingStatus().getStatus().equals(RecordingStatus.Recording)) {
+ LOG.info("Stopping recording");
+ SnapshotRecordResult recordResult = server.stopRecording();
+
+ List<StubMapping> stubMappings = recordResult.getStubMappings();
+ if (isDeleteRecordedMappingsOnError()) {
+ for (StubMapping mapping : stubMappings) {
+ int status = mapping.getResponse().getStatus();
+ if (status >= 300 && mapping.shouldBePersisted()) {
+ try {
+ String fileName = mapping.getName() + "-" + mapping.getId() + ".json";
+ Path mappingFilePath = Paths.get("./src/test/resources/mappings/", fileName);
+ Files.deleteIfExists(mappingFilePath);
+ LOG.infof("Deleted mapping file %s as status code was %d", fileName, status);
+ } catch (IOException e) {
+ LOG.errorf("Failed to delete mapping file %s", e, mapping.getName());
+ }
}
}
}
}
+ server.stop();
}
- server.stop();
+ } finally {
+ servers.remove(getServerKey());
Review comment:
> the TestResourceLifecycleManager instance on which start() and stop() are called different?
No it's the same.
I have tricked myself into thinking that caching was needed. It was something odd in the Geocoder tests that made me go down this path.
Let me rework things a little. I can probably drop the first commit entirely...
----------------------------------------------------------------
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] [camel-quarkus] ppalaga commented on a change in pull request #1988: WireMockTestResourceLifecycleManager improvements
Posted by GitBox <gi...@apache.org>.
ppalaga commented on a change in pull request #1988:
URL: https://github.com/apache/camel-quarkus/pull/1988#discussion_r517184827
##########
File path: integration-tests-support/wiremock/src/main/java/org/apache/camel/quarkus/test/wiremock/WireMockTestResourceLifecycleManager.java
##########
@@ -78,30 +82,35 @@
*/
@Override
public void stop() {
- if (server != null) {
- LOG.info("Stopping WireMockServer");
- if (server.getRecordingStatus().getStatus().equals(RecordingStatus.Recording)) {
- LOG.info("Stopping recording");
- SnapshotRecordResult recordResult = server.stopRecording();
-
- List<StubMapping> stubMappings = recordResult.getStubMappings();
- if (isDeleteRecordedMappingsOnError()) {
- for (StubMapping mapping : stubMappings) {
- int status = mapping.getResponse().getStatus();
- if (status >= 300 && mapping.shouldBePersisted()) {
- try {
- String fileName = mapping.getName() + "-" + mapping.getId() + ".json";
- Path mappingFilePath = Paths.get("./src/test/resources/mappings/", fileName);
- Files.deleteIfExists(mappingFilePath);
- LOG.infof("Deleted mapping file %s as status code was %d", fileName, status);
- } catch (IOException e) {
- LOG.errorf("Failed to delete mapping file %s", e, mapping.getName());
+ WireMockServer server = getServer();
+ try {
+ if (server != null) {
+ LOG.info("Stopping WireMockServer");
+ if (server.getRecordingStatus().getStatus().equals(RecordingStatus.Recording)) {
+ LOG.info("Stopping recording");
+ SnapshotRecordResult recordResult = server.stopRecording();
+
+ List<StubMapping> stubMappings = recordResult.getStubMappings();
+ if (isDeleteRecordedMappingsOnError()) {
+ for (StubMapping mapping : stubMappings) {
+ int status = mapping.getResponse().getStatus();
+ if (status >= 300 && mapping.shouldBePersisted()) {
+ try {
+ String fileName = mapping.getName() + "-" + mapping.getId() + ".json";
+ Path mappingFilePath = Paths.get("./src/test/resources/mappings/", fileName);
+ Files.deleteIfExists(mappingFilePath);
+ LOG.infof("Deleted mapping file %s as status code was %d", fileName, status);
+ } catch (IOException e) {
+ LOG.errorf("Failed to delete mapping file %s", e, mapping.getName());
+ }
}
}
}
}
+ server.stop();
}
- server.stop();
+ } finally {
+ servers.remove(getServerKey());
Review comment:
How is the caching supposed to work, when you remove the server on every stop()? I thought the cache was meant to keep the same server instance for multiple test classes referring to the same TestResourceLifecycleManager. Maybe you meant something different?
----------------------------------------------------------------
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] [camel-quarkus] jamesnetherton commented on pull request #1988: WireMockTestResourceLifecycleManager improvements
Posted by GitBox <gi...@apache.org>.
jamesnetherton commented on pull request #1988:
URL: https://github.com/apache/camel-quarkus/pull/1988#issuecomment-721669207
Thanks for the review, it helped to clear my head a bit :+1:
I'll close this and make d37e453 part of another PR where I plan port some itests over to WireMock.
----------------------------------------------------------------
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