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

rework selenium tests in organization package #6717

Merged
merged 2 commits into from
Oct 13, 2017

Conversation

Ohrimenko1988
Copy link
Contributor

What does this PR do?

Fix problem with duplicated organization tests in codenvy/codenvy , and add ability to run organization tests on the codenvy/codenvy

What issues does this PR fix or reference?

codenvy/codenvy#2483

Release Notes

Docs PR

@Ohrimenko1988 Ohrimenko1988 added kind/task Internal things, technical debt, and to-do tasks to be performed. status/code-review This issue has a pull request posted for it and is awaiting code review completion by the community. labels Oct 13, 2017
@@ -45,4 +49,12 @@ public Entrance getEntrance(
return new CookieEntrance(seleniumWebDriver);
}
}

@Provides
Copy link
Contributor

@dmytro-ndp dmytro-ndp Oct 13, 2017

Choose a reason for hiding this comment

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

Could you, please, explain, why did you place this provider here?
This module is dedicated for classes which are injecting SeleniumWebDriver, but I don't see such field in the TestOrganizationServiceClient, and it shouldn't be there according to the propose of that client.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Because in che we return CheTestTestOrganizationServiceClientImpl and in the codenvy we return other implementation of TestOrganizationServiceClient interface

Copy link
Contributor

Choose a reason for hiding this comment

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

I see. But the propose of CheSeleniumWebDriverRelatedModule is the classes which are injecting webdriver itself.

@dmytro-ndp
Copy link
Contributor

dmytro-ndp commented Oct 13, 2017

@Ohrimenko1988: there are new tests in PR like UserOrganizationTest, but I don't see any changes in tests suites. How are the new tests going to be started?

Copy link
Contributor Author

@Ohrimenko1988 Ohrimenko1988 left a comment

Choose a reason for hiding this comment

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

@dmytro-ndp tests can be started by using package running ( --test=organization.* )

@@ -45,4 +49,12 @@ public Entrance getEntrance(
return new CookieEntrance(seleniumWebDriver);
}
}

@Provides
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Because in che we return CheTestTestOrganizationServiceClientImpl and in the codenvy we return other implementation of TestOrganizationServiceClient interface

@dmytro-ndp
Copy link
Contributor

Do you have corresponding changes in the Codenvy repo as well?

@Ohrimenko1988
Copy link
Contributor Author

@dmytro-ndp yes I have prepared changes in the codenvy/codenvy according to changes in this PR

@dmytro-ndp
Copy link
Contributor

Which branch in Codenvy repo contains the changes?

@Ohrimenko1988
Copy link
Contributor Author

@dmytro-ndp
Copy link
Contributor

Thank you, @Ohrimenko1988.
OrganizationSuite.xml looks an obsolete. Lets remove it.

@Ohrimenko1988
Copy link
Contributor Author

@dmytro-ndp yes, you are absolutely right, thx

Copy link
Contributor

@dmytro-ndp dmytro-ndp left a comment

Choose a reason for hiding this comment

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

Looks good.

@Ohrimenko1988 Ohrimenko1988 merged commit 6019a0c into master Oct 13, 2017
@Ohrimenko1988 Ohrimenko1988 deleted the fix-organizations-tests branch October 13, 2017 12:21
@benoitf benoitf removed the status/code-review This issue has a pull request posted for it and is awaiting code review completion by the community. label Oct 13, 2017
@benoitf benoitf added this to the 5.19.0 milestone Oct 13, 2017
@codenvy-ci
Copy link

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/task Internal things, technical debt, and to-do tasks to be performed.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants