#1992 improve gitcontextmock#2093
Conversation
Coverage Report for CI Build 29928632798Coverage increased (+0.09%) to 72.586%Details
Uncovered ChangesNo uncovered changes found. Coverage RegressionsNo coverage regressions found. Coverage Stats
💛 - Coveralls |
maybeec
left a comment
There was a problem hiding this comment.
Thanks for tackling this — the mock now models the full clone → fetch → pull → update-detection lifecycle with a pending-commits queue, and CI is green. Nicely structured and the new GitContextMockTest reads well.
I have two blocking items that come straight from the issue text / project conventions, plus a scope question. Inline comments give one-click suggestions for the convention fix.
Must-fix
- Use the project's
IniFilesupport for the.git/configfile. #1992 explicitly asks that the config be created "with our IniFile support". Both the write (inclone) and the read (inretrieveGitUrl) are currently hand-rolled.com.devonfw.tools.ide.io.ini.IniFile/IniFileImplalready exist — using them also removes the fragile line parsing (see inline comments). - Exception handling convention.
coding-conventions.adoc(§"Catching and handling Exceptions") requires wrapping asthrow new IllegalStateException("<meaningful message>", e)— a message and the cause. There are 8 barethrow new RuntimeException(e)sites; I've left commit-able suggestions on each.
Scope question (non-blocking, worth confirming)
The issue closes with: "Our tests using GitContextMock can then be reworked and improved… Also we can properly test features like ide upgrade or the check if update is available." This PR delivers the mock infrastructure and self-tests, but the existing consumer (UpdateCommandletTest) isn't reworked and there are no feature-level tests exercising the new capabilities. Is that intended as a follow-up, or should it land here to fully close #1992?
Minor
GitChangehas noequals/hashCode, soGitCommit.hashCode()(which delegates toList<GitChange>.hashCode()) is identity-based — fine for a single run, just not content-stable. See inline note.java.io.IOExceptionFQN inclone/pullvs. the importedIOExceptionelsewhere is inconsistent (cosmetic).- Heads-up:
IdeTestContextdefaults toGitContextMock, so flipping these methods from no-ops to active FS operations changes semantics for every consumer (e.g.updateUrls()now creates a.gittree under the urls path). CI is green so nothing breaks today — just noting the widened side-effect surface.
Also needs a rebase — the branch is currently behind main.
maybeec
left a comment
There was a problem hiding this comment.
Requesting changes — there are blocking items that should be resolved before merge (details are in the inline comments on the previous review):
Blocking
- Use the project's
IniFilesupport for.git/config(write inclone, read inretrieveGitUrl). #1992 explicitly requires this, and it removes the fragile hand-rolled parsing that currently isn't scoped to the[remote "origin"]section. - Exception handling convention — replace the 8
throw new RuntimeException(e)sites withIllegalStateException("<message>", e)(message + cause), percoding-conventions.adoc. One-click suggestions are provided on each line.
Please also clarify (scope of #1992)
- The issue asks that the existing tests using
GitContextMockbe reworked and that features likeide upgrade/ update-detection be properly tested with the new mock. That part isn't in this PR — is it a planned follow-up, or should it land here to fully close #1992?
Once the two blocking items are addressed (and the branch rebased onto main), I'm happy to re-review. Thanks again — the core lifecycle modelling is solid.
7fe0b65 to
0d37f75
Compare
Thanks for the review. I addressed the two blocking items:
I also added equals/hashCode for GitChange and GitCommit to address the minor hash stability note. Regarding the scope question: I intended this PR to focus on the GitContextMock infrastructure and its self-tests. Reworking consumer tests such as UpdateCommandletTest would be a follow-up, unless you prefer to include it here. |
Added a new entry for the 2026.07.002 release, highlighting improvements to GitContextMock.
…/1992-improve-gitcontextmock # Conflicts: # CHANGELOG.adoc # cli/src/test/java/com/devonfw/tools/ide/git/GitContextMock.java
- Improve GitContextMock implementation - Use FileAccess for file operations
Re-added the entry for improving localization of GUI in the changelog.
This PR fixes #1992
Implemented changes:
Testing instructions
Please add conscise, understandable instructions on how a reviewer can test/verify the functionality of your contribution here:
GitContextMockTest.Checklist for this PR
Make sure everything is checked before merging this PR. For further info please also see
our DoD.
mvn clean testlocally all tests pass and build is successful#«issue-id»: «brief summary»(e.g.#921: fixed setup.bat). If no issue ID exists, title only.In Progressand assigned to you or there is no issue (might happen for very small PRs)with
internal