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

[JENKINS-32777] Cover 'delete-builds' CLI by tests #2310

Merged
merged 3 commits into from
May 12, 2016

Conversation

pjanouse
Copy link
Contributor

@pjanouse pjanouse commented May 4, 2016

Please review, thx. Pavel


@Test public void deleteBuildsShouldSuccessEvenTheBuildIsRunning() throws Exception {
FreeStyleProject project = j.createFreeStyleProject("aProject");
project.getBuildersList().add(new Shell("echo 1\nsleep 10s"));
Copy link
Member

Choose a reason for hiding this comment

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

This is fragile. Something like ExecutorTest.BlockingBuilder should be used.

@daniel-beck
Copy link
Member

LGTM


@Test public void deleteBuildsManyShouldSuccessEvenTheFirstAndLastBuildDoesNotExist() throws Exception {
FreeStyleProject project = j.createFreeStyleProject("aProject");
project.scheduleBuild2(0).get();
Copy link
Member

Choose a reason for hiding this comment

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

j.buildAndAssertSuccess would hide some implementation details. Not important

@oleg-nenashev
Copy link
Member

👍

@oleg-nenashev oleg-nenashev added the ready-for-merge The PR is ready to go, and it will be merged soon if there is no negative feedback label May 10, 2016
@oleg-nenashev
Copy link
Member

@olivergondza Regarding your comment... Do you want to get it fixed before the merge?

@pjanouse
Copy link
Contributor Author

@oleg-nenashev Yes, I'll fix it before merge.

@oleg-nenashev oleg-nenashev added work-in-progress The PR is under active development, not ready to the final review and removed ready-for-merge The PR is ready to go, and it will be merged soon if there is no negative feedback labels May 10, 2016
@oleg-nenashev oleg-nenashev added needs-more-reviews Complex change, which would benefit from more eyes and removed work-in-progress The PR is under active development, not ready to the final review labels May 11, 2016
@oleg-nenashev
Copy link
Member

👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs-more-reviews Complex change, which would benefit from more eyes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants