-
-
Notifications
You must be signed in to change notification settings - Fork 314
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Updated tomcat playlist to copy the results from docker container #605
Conversation
@chandrams - Any update on this? |
Festival of Lights in India, I suspect we will get an update on Thursday or after. :) |
@Mesbah-Alam - Tomcat output logs aren't in the junit format. I have added the changes to copy the result logs though, let me know if you want to retain them. |
What format are they? Some standard test output format? |
Sorry I checked the wrong logs, they are in .txt format and I think they use junit. cat TEST-org.apache.tomcat.websocket.TestWsSession.NIO.txt Testcase: testAppendCloseReasonWithTruncation01 took 0.016 sec |
hi @chandrams - would you be able to update this PR to resolve conflicts? Then we can verify in a Grinder. Thanks! |
8ac977b
to
2bad867
Compare
Thanks Shelley @smlambert, I have resolved the conflicts. |
May I request a review from you @sophia-guo as you have been working on external tests recently? |
@@ -15,7 +15,9 @@ | |||
<playlist xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:noNamespaceSchemaLocation="../TestConfig/playlist.xsd"> | |||
<test> | |||
<testCaseName>tomcat_test</testCaseName> | |||
<command>docker run --rm $(EXTRA_DOCKER_ARGS) adoptopenjdk-tomcat-test:latest ; \ | |||
<command>docker run --name tomcat-test $(EXTRA_DOCKER_ARGS) adoptopenjdk-tomcat-test:latest ; \ | |||
docker cp tomcat-test:/tomcat/output/build/logs $(REPORTDIR)/external_test_reports; \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would suggest moving docker cp and docker rm
after ${TEST_STATUS}
like other tests. Otherwise the ${TEST_STATUS} will be determined by the status of docker rm
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also if tests pass
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks Sophia have made the change. Please review.
@chandrams - when you get a chance, please update with @sophia-guo 's suggestions. Thanks! |
Signed-off-by: Chandrakala M Subramanyam <[email protected]>
2bad867
to
b638d6a
Compare
@sophia-guo - ready for another review |
Thanks @chandrams |
Updated playlist to copy the results from docker container
issue #976
Signed-off-by: Chandrakala [email protected]