Windows smoke tests for Jetty, Tomcat, GlassFish #87
Conversation
smoke-tests/src/test/java/com/splunk/opentelemetry/ProprietaryAppServerTest.java
Outdated
Show resolved
Hide resolved
|
Quick question: why |
| private static final Logger logger = LoggerFactory.getLogger(WindowsTestContainerManager.class); | ||
|
|
||
| private static final String NPIPE_URI = "npipe:////./pipe/docker_engine"; | ||
| private static final String COLLECTOR_CONFIG_FILE_PATH = "/collector-config.yml"; |
There was a problem hiding this comment.
Why do we need this to differ from AbstractTestContainerManager#COLLECTOR_CONFIG_RESOURCE?
There was a problem hiding this comment.
The difference in values is not actually necessary, but I think separate fields still make sense, because they are used in different places - the RESOURCE one refers to the filename in the test code classpath, and the PATH refers to the filename used in container filesystem.
There was a problem hiding this comment.
And that PATH is only relevant for Windows-based images?
There was a problem hiding this comment.
The path only needs to be consistent across the copy to container operation and command used to launch the collector, which are both done in the same class. Therefore it is an internal implementation details of the container manager and there is no need to expose it to any other class.
smoke-tests/src/test/java/com/splunk/opentelemetry/AppServerTest.java
Outdated
Show resolved
Hide resolved
| } | ||
| } | ||
|
|
||
| public static class Http extends TargetWaitStrategy { |
There was a problem hiding this comment.
As the logic for waiting behind http endpoint was internally already implemented as it was used for fake backend, it made sense to keep the list of wait strategies in sync with the capabilities that are already implemented anyway.
smoke-tests/src/test/java/com/splunk/opentelemetry/SpringBootSmokeTest.java
Outdated
Show resolved
Hide resolved
smoke-tests/src/test/java/com/splunk/opentelemetry/SpringBootSmokeTest.java
Outdated
Show resolved
Hide resolved
smoke-tests/src/test/java/com/splunk/opentelemetry/TomcatSmokeTest.java
Outdated
Show resolved
Hide resolved
|
Will other app servers follow to Windows as well? |
I think we have to look at the wider picture here - upstreaming just fake-backend is not very useful without at least one test that would use it. But for that we also would need the windows-dockerized collector. These could be follow up steps within the effort of open-sourcing the test matrix. |
It could, but then that would be a blocker for this, and I'm not sure if upstreaming the backend alone would make sense until some Windows tests are upstreamed as well.
Pretty much just adapt all the additional logic in both |
|
Rebase this PR please |
agoallikmaa
force-pushed
the
windows-test-images
branch
from
1d6499d
to
bca9f8d
Compare
Done. |
I do not see any reason why not, but they are not in the scope of my current task. |
…pps from Tomcat windows image
Adds tasks and Dockerfiles to build Windows Docker images for:
Implements an alternative way of test container management that works with Windows images (by using docker-java directly) as Testcontainers does not support Windows images. Platform and type (proprietary or not) are now also specified for images used in smoke tests to determine which images can be tested with on the current system.
Running Windows smoke tests also added to CI workflow, where for now, the Windows images are built locally before running tests.