Skip to content
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

[JBIDE-23648] fixed ImageTagTest#addUpperCaseTagToImage #1708

Merged
merged 1 commit into from Dec 16, 2016

Conversation

adietish
Copy link
Member

No description provided.

@adietish
Copy link
Member Author

@jkopriva please review

@rhopp
Copy link
Member

rhopp commented Dec 15, 2016

I don't think is a good move. I would leave this test failing until it is fixed in tooling (or upstream?). Artificially marking failing test as passing induces high risk of omitting to undo this change when the issue is fixed.

@adietish
Copy link
Member Author

adietish commented Dec 15, 2016

@rhopp @jeffmaury the aim here is to make the suite blue, as we discussed and agreed in the last meeting.
When using the @test(expected=) approach the test will actually start to fail once the docker tooling is fixed upstream. So there is no danger this is going to be forgotten. On the contrary, it catches and asserts the expected current state of the tooling.

@jeffmaury
Copy link
Member

@rhopp @adietish I agree with André the goal is to have a test suite blue for automated testing and using expected clause will make the test to fail when the issue will be fixed. Anything that does require an light or heavy analysis of the results will fail because dev will then no use those tests

@adietish
Copy link
Member Author

It looks as if we should only expect the error dialog not to show up if the docker daemon being used is < 1.11. See https://bugs.eclipse.org/bugs/show_bug.cgi?id=509223#c2

@adietish
Copy link
Member Author

so for now we decided to assert the tagging error (when using uppercase tags) for docker daemons >= 1.11, while we dont for the ones below. The test will get green.
Whenever something is moving in the Eclipse Docker tooling we'll notice and be able to catch up in our test.

@adietish adietish force-pushed the JBIDE-23648 branch 2 times, most recently from 8b78a5a to 42a5ea3 Compare December 16, 2016 14:21
@jkopriva
Copy link
Member

@rhopp @psrna fix is working, please review and merge

@rhopp
Copy link
Member

rhopp commented Dec 16, 2016

Merging without verifcation job (there is some problem in jenkins with provisioning slaves).

@rhopp rhopp merged commit d56f3b3 into jbosstools:master Dec 16, 2016
@adietish adietish deleted the JBIDE-23648 branch December 16, 2016 22:48
theJNOVAK pushed a commit to theJNOVAK/jbosstools-integration-tests that referenced this pull request Jan 3, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants