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.

Sign up for GitHub

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

Jump to bottom

Add auxiliary test github user repository #9308

Merged
merged 3 commits into from
Apr 2, 2018
Merged

Add auxiliary test github user repository #9308

merged 3 commits into from
Apr 2, 2018

Conversation

dmytro-ndp
Copy link
Contributor

@dmytro-ndp dmytro-ndp commented Apr 2, 2018 •
edited
Loading

What does this PR do?

  • adds java docs to TestGitHubRepository class
  • add ability to inject test github repository as a test github auxiliary user.

How to use:

  @Inject
  @Named(CheSeleniumSuiteModule.AUXILIARY)
  private TestGitHubRepository testAuxiliaryRepo;

What issues does this PR fix or reference?

#9155

Dmytro Nochevnov added 2 commits March 30, 2018 18:32
Add java docs to TestGitHubRepository class
e6fded6 
Signed-off-by: Dmytro Nochevnov <dnochevnov@codenvy.com>
Add auxiliary github user repository
a630bee 
Signed-off-by: Dmytro Nochevnov <dnochevnov@codenvy.com>
@dmytro-ndp dmytro-ndp added status/code-review This issue has a pull request posted for it and is awaiting code review completion by the community. kind/task Internal things, technical debt, and to-do tasks to be performed. team/che-qe labels Apr 2, 2018
Ohrimenko1988
Ohrimenko1988 reviewed Apr 2, 2018
View reviewed changes
...e-selenium-core/src/main/java/org/eclipse/che/selenium/core/client/TestGitHubRepository.java
private void createFile(Path pathToRootContentDirectory, Path pathToFile) throws IOException {
byte[] contentBytes = Files.readAllBytes(pathToFile);
String relativePath = pathToRootContentDirectory.relativize(pathToFile).toString();
String commitMessage = String.format("Add file %s", relativePath);
Copy link
Contributor

Choose a reason for hiding this comment

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

String commitMessage = "Add file " + relativePath;

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I would prefer String.format()

Copy link
Contributor

Choose a reason for hiding this comment

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

But in my opinion in this case the concatenation is better for use. But of course, this is only my opinion.

Ohrimenko1988
Ohrimenko1988 reviewed Apr 2, 2018
View reviewed changes
...-selenium-test/src/test/java/org/eclipse/che/selenium/git/PullRequestPluginWithForkTest.java Outdated
@@ -60,6 +60,12 @@
@Inject private Ide ide;
@Inject private TestUser productUser;

// @Inject private TestGitHubRepository testRepo;

Copy link
Contributor

Choose a reason for hiding this comment

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

Why commenting is used?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is an example of used in target test.

Copy link
Contributor

Choose a reason for hiding this comment

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

Should you uncomment this rows? And in this case test maintainer should only check and approve your changes and doesn`t do anything more.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If I uncomment it for now, it will be creating two unnecessary repositories during the execution while the test will be fixed.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I would remove that code and add en example in the PR description.

Remove redundant commented code
2621048 
Signed-off-by: Dmytro Nochevnov <dnochevnov@codenvy.com>
@dmytro-ndp dmytro-ndp merged commit a42e873 into master Apr 2, 2018
@dmytro-ndp dmytro-ndp deleted the che-9155 branch April 2, 2018 08:55
@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 Apr 2, 2018
@benoitf benoitf added this to the 6.4.0 milestone Apr 2, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@musienko-maxim musienko-maxim musienko-maxim approved these changes

@artaleks9 artaleks9 artaleks9 approved these changes

@SkorikSergey SkorikSergey SkorikSergey approved these changes

@Ohrimenko1988 Ohrimenko1988 Ohrimenko1988 approved these changes

@vparfonov vparfonov Awaiting requested review from vparfonov

Assignees
No one assigned
Labels
kind/task Internal things, technical debt, and to-do tasks to be performed.
Projects
None yet
Milestone
6.4.0
Development

Successfully merging this pull request may close these issues.

None yet
6 participants
@dmytro-ndp @musienko-maxim @artaleks9 @SkorikSergey @Ohrimenko1988 @benoitf