Skip to content

#1988: improvment of url updater tests#2012

Open
oanding-blrng wants to merge 10 commits into
devonfw:mainfrom
oanding-blrng:feature/1988-improvment-of-url-updater-tests
Open

#1988: improvment of url updater tests#2012
oanding-blrng wants to merge 10 commits into
devonfw:mainfrom
oanding-blrng:feature/1988-improvment-of-url-updater-tests

Conversation

@oanding-blrng

@oanding-blrng oanding-blrng commented Jun 9, 2026 •
edited
Loading

Copy link
Copy Markdown

This PR fixes #1988: Refactored UrlUpdaterTests to not use mock classes

Implemented changes:

  • Modified AbstractUrlUpdater to receive downloadBaseUrl and versionBaseUrl
  • Modified corresponding UrlUpdater for GitHub, IdeaBased, Json, etc.
  • Refactored all tool specific url updater classes to declare up to two constants DOWNLOAD_BASE_URL and VERSION_BASE_URL and and pass them up the constructors
  • Added package-private constructors to all tool url updaters (which had already url updater tests)
  • Refactored all tool url updater tests to not override getDownloadBaseUrl() and getVersionBaseUrl() and make use of new constructor to pass mock urls
  • Removed all url updater mock classes, which weren't needed anymore

Testing instructions

Please add conscise, understandable instructions on how a reviewer can test/verify the functionality of your contribution here:

  1. Switch to main Branch and run mvn -pl url-updater -am clean test
  2. Switch to feature Branch and run mvn -pl url-updater -am clean test (Time of writing: all 70 tests should run successfully)
  3. Toggle off the internet connection and rerun the tests. (No tests tries to call real endpoints and all are using mock urls).

Checklist for this PR

Make sure everything is checked before merging this PR. For further info please also see
our DoD.

  • When running mvn clean test locally all tests pass and build is successful
  • PR title is of the form #«issue-id»: «brief summary» (e.g. #921: fixed setup.bat). If no issue ID exists, title only.
  • PR top-level comment summarizes what has been done and contains link to addressed issue(s)
  • PR and issue(s) have suitable labels
  • Issue is set to In Progress and assigned to you or there is no issue (might happen for very small PRs)
  • You followed all coding conventions
  • You have added the issue implemented by your PR in CHANGELOG.adoc unless issue is labeled
    with internal
  • You have formulated clear instructions on how to test your contribution under "Testing instructions"

@oanding-blrng
devonfw#1988 Example implementation of refactoring.
a39a513 
- Extended AbstractUrlUpdater with new private final field and added constructor
- Extended JsonUrlUpdater and JavaUrlUpdater with additional constructors
- Updated JavaUrlUpdaterTest to use JavaUrlUpdater without the mock and deleted JavaUrlUpdaterMock.java
@oanding-blrng
devonfw#1988 Moved GoUrlUpdater into proper package to follow project…
3acd102 
… structure.
@oanding-blrng
devonfw#1988 Updated UrlUpdaters within url updater module.
e1a51fe 
- Refactored DOWNLOAD_BASE_URL and VERSION_BASE_URL according to ticket description in files where changes there necessary.
@oanding-blrng
devonfw#1988 Updated url updater tests for several tools.
778825f
@oanding-blrng
Merge branch 'main' into feature/1988-improvment-of-url-updater-tests
c0ca75a
@oanding-blrng
devonfw#1988 Modified many constructors of tool url updaters to reall…
185b116 
…y use mock URLs instead of production ones. Also added comments to constructors.
@oanding-blrng
devonfw#1988 Deleting all the mocks of url updaters, which aren't nee…
72fda7c 
…ded anymore.
@oanding-blrng
devonfw#1988 Updated Javadoc on constructors used for testing.
6404b59
@oanding-blrng
devonfw#1988 Updated Javadoc on constructors in AbstractGithubUrlUpda…
5e0ecb7 
…ter.
@github-project-automation github-project-automation Bot moved this to 🆕 New in IDEasy board Jun 9, 2026
@CLAassistant

CLAassistant commented Jun 9, 2026 •
edited
Loading

Copy link
Copy Markdown

CLA assistant check
All committers have signed the CLA.

@coveralls

coveralls commented Jun 9, 2026 •
edited
Loading

Copy link
Copy Markdown
Collaborator

Coverage Report for CI Build 27545523906

Warning

Build has drifted: This PR's base is out of sync with its target branch, so coverage data may include unrelated changes.
Quick fix: rebase this PR. Learn more →

Coverage decreased (-0.009%) to 71.043%

Details

  • Coverage decreased (-0.009%) from the base build.
  • Patch coverage: No coverable lines changed in this PR.
  • 371 coverage regressions across 53 files.

Uncovered Changes

No uncovered changes found.

Coverage Regressions

371 previously-covered lines in 53 files lost coverage.

Top 10 Files by Coverage Loss Lines Losing Coverage Coverage
com/devonfw/tools/ide/url/updater/AbstractUrlUpdater.java 38 84.59%
com/devonfw/tools/ide/url/tool/eclipse/EclipseUrlUpdater.java 25 0.0%
com/devonfw/tools/ide/url/tool/docker/DockerDesktopUrlUpdater.java 17 0.0%
com/devonfw/tools/ide/url/tool/dotnet/DotNetUrlUpdater.java 16 0.0%
com/devonfw/tools/ide/url/tool/mvn/MvnUrlUpdater.java 16 0.0%
com/devonfw/tools/ide/url/updater/MavenBasedUrlUpdater.java 16 0.0%
com/devonfw/tools/ide/url/tool/node/NodeUrlUpdater.java 14 0.0%
com/devonfw/tools/ide/url/tool/helm/HelmUrlUpdater.java 11 0.0%
com/devonfw/tools/ide/url/tool/java/JavaUrlUpdater.java 11 68.52%
com/devonfw/tools/ide/url/tool/kotlinc/KotlincUrlUpdater.java 11 0.0%

Coverage Stats

Coverage Status
Relevant Lines: 15908
Covered Lines: 11794
Line Coverage: 74.14%
Relevant Branches: 7074
Covered Branches: 4533
Branch Coverage: 64.08%
Branches in Coverage %: Yes
Coverage Strength: 3.13 hits per line
💛 - Coveralls

@oanding-blrng oanding-blrng self-assigned this Jun 9, 2026
@oanding-blrng oanding-blrng added enhancement New feature or request test related to testing and QA urls ide-urls repo and related processes and features internal Nothing to be added to CHANGELOG, only internal story labels Jun 9, 2026
@oanding-blrng oanding-blrng moved this from 🆕 New to Team Review in IDEasy board Jun 9, 2026
@oanding-blrng oanding-blrng marked this pull request as ready for review June 9, 2026 14:32
tineff96
tineff96 reviewed Jun 12, 2026
View reviewed changes

@tineff96 tineff96 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Very nice refactoring overall! Moving the download/version base URLs into constructor parameters makes the updater classes much easier to test with the real production implementations and removes a lot of dedicated mock subclasses. This clearly took quite some dedication.

One small robustness question: would AbstractUrlUpdater be the right place to validate the new base URL constructor arguments? I mainly see this as a possible issue for future tests or subclasses passing custom WireMock URLs. If invalid values such as null or blank strings are not guarded elsewhere, failing fast in the constructor could make misconfigurations easier to diagnose.

@hohwille hohwille changed the title Feature/1988 improvment of url updater tests #1988: improvment of url updater tests Jun 15, 2026
@oanding-blrng

oanding-blrng commented Jun 15, 2026

Copy link
Copy Markdown
Author

I see your point. I added validation of the arguments passed to the constructor of AbstractUrlUpdater. It will now check for null and "" values. If this is the case, an IllegalArgumentException with further explanation will be thrown, because it doesn't make sense to continue execution at this point.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@tineff96 tineff96 tineff96 approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@tineff96 tineff96

@oanding-blrng oanding-blrng

Labels

enhancement New feature or request internal Nothing to be added to CHANGELOG, only internal story test related to testing and QA urls ide-urls repo and related processes and features

Projects

IDEasy board
Status: Team Review

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Improve UrlUpdater tests

4 participants

@oanding-blrng @CLAassistant @coveralls @tineff96