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

[JENKINS-63572] Avoid NPE if remote config is empty #954

Merged
merged 3 commits into from
Sep 12, 2020
Merged

[JENKINS-63572] Avoid NPE if remote config is empty #954

merged 3 commits into from
Sep 12, 2020

Conversation

MarkEWaite
Copy link
Contributor

JENKINS-63572 - Avoid NPE if remote config is empty

If a pipeline job defines the checkout scm step without including a remote repository, the job will usually fail in other ways.

A null pointer exception is not a friendly way to show a configuration error to a user.

Checklist

  • I have read the CONTRIBUTING doc
  • I have referenced the Jira issue related to my changes in one or more commit messages
  • I have added tests that verify my changes
  • Unit tests pass locally with my changes
  • I have added documentation as necessary
  • No Javadoc warnings were introduced with my changes
  • No spotbugs warnings were introduced with my changes
  • Documentation in README has been updated as necessary
  • Online help has been added and reviewed for any new or modified fields
  • Any dependent changes have been merged and published in upstream modules (like git-client-plugin)

Types of changes

  • Bug fix (non-breaking change which fixes an issue)

@MarkEWaite
[JENKINS-63572] test for null pointer exception
e71936e 
GitToolChooser should not report a null pointer exception, even when
the job definition is incorrect.
@MarkEWaite
[JENKINS-63572] Avoid NPE if remote configs is empty
be5485d 
If a pipeline job defines the checkout scm step without including a
remote repository, the job will usually fail in other ways.

A null pointer exception is not a friendly way to show a configuration
error to a user.
@MarkEWaite
Remove useless trailing white space
9b36b09 
Pipeline example in test does not need
trailing white space
rishabhBudhouliya
rishabhBudhouliya approved these changes Sep 12, 2020
View reviewed changes
Copy link
Contributor

@rishabhBudhouliya rishabhBudhouliya left a comment

Choose a reason for hiding this comment

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

While the check on chooser being null is sufficient, I wanted to ask if the GitToolChooser should know better and dodge an empty or a null URL string?

@MarkEWaite
Copy link
Contributor Author

While the check on chooser being null is sufficient, I wanted to ask if the GitToolChooser should know better and dodge an empty or a null URL string?

I'm not sure what a git tool chooser would mean when a remote repository is not provided. Without a remote repository, there is nothing to fetch.

@rishabhBudhouliya
Copy link
Contributor

@MarkEWaite Agreed, thanks for resolving this issue + the test!

@MarkEWaite MarkEWaite merged commit 22be427 into jenkinsci:master Sep 12, 2020
@MarkEWaite MarkEWaite deleted the fix-npe-on-empty-remote-config branch September 12, 2020 19:08
@MarkEWaite MarkEWaite added the bugfix Fixes a bug - used by Release Drafter label Sep 12, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rishabhBudhouliya rishabhBudhouliya rishabhBudhouliya approved these changes

Assignees
No one assigned
Labels
bugfix Fixes a bug - used by Release Drafter
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@MarkEWaite @rishabhBudhouliya