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

Remove two Assembla tests #1144

Merged
merged 1 commit into from
Sep 16, 2021
Merged

Remove two Assembla tests #1144

merged 1 commit into from
Sep 16, 2021

Conversation

MarkEWaite
Copy link
Contributor

BOM PR 650 - Remove two broken Assembla tests

Tests now fail due to changes in the https://www.assembla.com web site. Not enough value in those tests to retain them. Will reduce code coverage but also will reduce maintainence overhead and provide a small reduction in test time.

Checklist

  • I have read the CONTRIBUTING doc
  • I have removed 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

  • Test

@MarkEWaite
Remove two Assembla tests
3c4e430 
Fail due to changes in the https://www.assembla.com web site.  Not
enough value in those tests to retain them.  Will reduce code coverage
but also will reduce maintainence overhead and provide a small
reduction in test time.
MarkEWaite added a commit to MarkEWaite/bom that referenced this pull request Sep 16, 2021
@MarkEWaite
Ignore failing git plugin Assembla test
7d1d332 
Test depends on the https://www.assembla.com web site.  Web site layout
has changed enough that the test now fails.

Test is removed from the plugin in jenkinsci/git-plugin#1144
@MarkEWaite MarkEWaite mentioned this pull request Sep 16, 2021
Merged
jglick
jglick approved these changes Sep 16, 2021
View reviewed changes
Copy link
Member

@jglick jglick left a comment

Choose a reason for hiding this comment

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

Or, since presumably real users would start seeing form validation errors here, actually fix doCheckRepoUrl to omit the content check? (Or adapt it to the new content, while removing the test cases that cannot be reliable?)

@MarkEWaite
Copy link
Contributor Author

Agreed @jglick that I'll likely remove all the content checks for Assembla in a future pull request.

@MarkEWaite MarkEWaite merged commit a526de4 into jenkinsci:master Sep 16, 2021
@MarkEWaite MarkEWaite deleted the remove-assembla-tests branch September 16, 2021 18:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@timja timja timja approved these changes

@jglick jglick jglick approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@MarkEWaite @jglick @timja