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

Fixed NPE during provisioning of selector for Deployments #13041

Merged
merged 2 commits into from
Apr 2, 2019

Conversation

sleshchenko
Copy link
Member

What does this PR do?

This bug was found during try to run a workspace with a single dockerimage component.
It fixes provisioning of selector for Deployment of dockerimage component and improves the code to avoid NPE if the user does not define selector in his recipe.

What issues does this PR fix or reference?

It is a fix for functionality introduced in #12967

Release Notes

N/A

Docs PR

N/A

@sleshchenko sleshchenko added kind/bug Outline of a bug - must adhere to the bug report template. status/code-review This issue has a pull request posted for it and is awaiting code review completion by the community. labels Apr 2, 2019
@sleshchenko sleshchenko self-assigned this Apr 2, 2019
@sleshchenko
Copy link
Member Author

ci-test

Copy link
Contributor

@metlos metlos left a comment

Choose a reason for hiding this comment

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

LGTM but makes me worried where else do we assume k8s child objects to be non-null.

@che-bot
Copy link
Contributor

che-bot commented Apr 2, 2019

Results of automated E2E tests of Eclipse Che Multiuser on OCP:
Build details
Test report
docker image: eclipseche/che-server:13041
https://github.com/orgs/eclipse/teams/eclipse-che-qa please check this report.

@vkuznyetsov vkuznyetsov mentioned this pull request Apr 2, 2019
12 tasks
@SkorikSergey
Copy link
Contributor

Selenium tests execution on Eclipse Che Multiuser on OCP (https://ci.codenvycorp.com/job/che-pullrequests-test-ocp/1678//Selenium_20tests_20report/) doesn't show any regression against this Pull Request.

…component

Signed-off-by: Sergii Leshchenko <sleshche@redhat.com>
Signed-off-by: Sergii Leshchenko <sleshche@redhat.com>
@sleshchenko sleshchenko merged commit 48d97fc into eclipse-che:master Apr 2, 2019
@sleshchenko sleshchenko deleted the selectorNPEfix branch April 2, 2019 13:47
@sleshchenko
Copy link
Member Author

@rhopp Hi, this PR fixed regression introduced in #12967. Do we have a chance to get basic Devfile functionalities covered with selenium tests?

@rhopp
Copy link
Contributor

rhopp commented Apr 3, 2019

@sleshchenko I don't see much value in trying to cover devfile functionality with selenium tests in this moment... Selenium tests are UI tests and AFAIK there is no UI endpoint for devfile functionality, right?
Of course we could add workspace creation from devfile (by api call) into the cypress suite which is now in development, but beware, that this way we could cover only very limited number of possible "devfiles" (in the sense that we won't be able to test this way all possible devfile features and their combinations).

@sleshchenko
Copy link
Member Author

@rhopp You're right that there is not Devfile specific UI, and there are two ways of creation workspace with Devfile - factory URL and API call. My point is to have some integration tests that make sure that no functionalities are broken.

(in the sense that we won't be able to test this way all possible devfile features and their combinations).

something is better than nothing =)

@rhopp
Copy link
Contributor

rhopp commented Apr 3, 2019

@sleshchenko Could you please check #12728 (most importantly https://hackmd.io/s/BkVyU6crN) to see if that covers the basics for devfile and if not, could you provide feedback there? Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/bug Outline of a bug - must adhere to the bug report template. status/code-review This issue has a pull request posted for it and is awaiting code review completion by the community.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants