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-57775][JENKINS-62780] Handle PRs from/to non existing src/dest #322

Merged
merged 7 commits into from
Jul 17, 2020

Conversation

Dohbedoh
Copy link
Contributor

@Dohbedoh Dohbedoh commented Jun 24, 2020

Fix NPEs caused by deleted source and destination branch or repository of PRs.

Tested the different scenario against Bitbucket Cloud and confirmed that:

  • source.repository can be null (deleted source repository)
  • source.commit can be null (deleted source repository)
  • destination.commit can be null (deleted destination branch)

I haven't been able to reproduce a nullable source.branch or destination.branch.

When testing against Bitbucket Server (7.2.1), I could not reproduce such scenario:

  • PR opened from deleted source branch or forked are automatically declined.
  • Bitbucket Server does not let me delete a destination branch if there is an open PR against it: "You cannot delete this branch since it has at least one open pull request associated with it"

Your checklist for this pull request

  • Make sure you are requesting to pull a topic/feature/bugfix branch (right side) and not your master branch!
  • Ensure that the pull request title represents the desired changelog entry
  • Please describe what you did
  • Link to relevant issues in GitHub or in Jenkins JIRA
  • Link to relevant pull requests, esp. upstream and downstream changes
  • Did you provide a test-case? That demonstrates feature works or fixes the issue.

Copy link
Contributor

@bitwiseman bitwiseman left a comment

Choose a reason for hiding this comment

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

Looks like you ran into some spotbugs issues.

When testing against Bitbucket Server (7.2.1), I could not reproduce such scenario:

  • PR opened from deleted source branch or forked are automatically declined.
  • Bitbucket Server does not let me delete a destination branch if there is an open PR against it: "You cannot delete this branch since it has at least one open pull request associated with it"

Since you're controlling/mocking the responses, you could create a custom one that has these behaviors right?
Regardless of whether you can get the server to do it currently, it seems like something that should be tested to ensure it is handled.

if(this.branch != null) {
// redound available the informations into impl objects
if(this.branch != null && this.commit != null) {
// redound available the information into impl objects
Copy link
Contributor

Choose a reason for hiding this comment

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

"redound"?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's from #152. I am gonna rephrase this to Make the commit information available into impl objects. cc @nfalco79

Copy link
Member

Choose a reason for hiding this comment

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

It should was " re bind"

Copy link
Member

Choose a reason for hiding this comment

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

@bitwiseman I want heavy remember to you branch-api-plugin#160

Copy link
Contributor

Choose a reason for hiding this comment

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

@nfalco79 I hear you.

@Dohbedoh
Copy link
Contributor Author

Since you're controlling/mocking the responses, you could create a custom one that has these behaviors right?

I am not sure that those behaviors are even possible from Bitbucket Server.

@Dohbedoh Dohbedoh marked this pull request as ready for review June 29, 2020 12:30
@bitwiseman bitwiseman merged commit 57d869c into jenkinsci:master Jul 17, 2020
@Dohbedoh Dohbedoh deleted the bugfix/JENKINS-62780 branch July 30, 2020 03:45
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.

3 participants