Skip to content

Keep every Gradle module's dependencies when building the tree - #190

Merged
Jordanh1996 merged 10 commits into
masterfrom
fix/XRAY-100231-gradle-node-merge
Sep 27, 2026
Merged

Jordanh1996 merged 10 commits into
masterfrom
fix/XRAY-100231-gradle-node-merge

Conversation

@Jordanh1996

@Jordanh1996 Jordanh1996 commented Sep 22, 2026 •

Copy link
Copy Markdown
Contributor

Background

Scanning a multi-module Gradle project in the IntelliJ plugin fails with a NullPointerException when two modules resolve the same dependency differently.

Description

GradleTreeBuilder merges the per-module dependency trees instead of replacing them, and exposes each module's own tree via DepTree.modules(), used by jfrog/jfrog-idea-plugin#532. SarifParser now skips scanners it doesn't support instead of failing.

Tests

New GradleTreeBuilderTest case. The JfrogCliDriverTest audits scan a temp copy of their fixture, since jf ≥ 2.106.0 skips projects under a test path (XRAY-158874).


  • All tests passed. If this feature is not already covered by the tests, I added new tests.

Jordanh1996 and others added 4 commits September 22, 2026 20:10
A component that appears in more than one module's dependency-tree file may be
resolved there with different transitive dependencies. Replacing the node let the
last file win, which could leave a transitive dependency with no parent at all and
crash impact-graph construction.
Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
The prior assertions on 'modb' were vacuously true whether or not
module scopes aliased the merged node map, since 'modb' never
resolves commons-lang3 in its own file. Assert on modb's own
commons-text node's children instead, which does regress if a
module's DepTreeNode is aliased to the merged map's node.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Sep 22, 2026 •

Copy link
Copy Markdown

Important

  • 🔍 Trigger review

This repository does not receive automatic reviews because it has fewer than 10 stars.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Advanced

Run ID: 208bd80e-5e78-47c1-a3a6-62a66dedccae


Comment @coderabbitai help to get the list of available commands.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Read each module's dependency-tree file into its own module tree first, then merge
those, so each step reads on its own. Fold the two shared-dependency tests into one
and drop a test that only restated the DepTree constructor.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Since jf 2.106.0, jf audit matches its exclusion patterns against the absolute path of
the working directory, so the default *test* pattern skips any project under src/test
and the audit returns an empty SARIF. Copy the fixture to a temporary directory first.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
jf audit now also reports a JFrog Services scanner run. SourceCodeScanType has no value
for it, so the whole SARIF failed to parse. Skip unsupported runs instead, and make the
npm audit test assert on its SCA finding rather than on the number of files with
findings, which grows with the scanners the server runs.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@Jordanh1996
Jordanh1996 marked this pull request as ready for review September 24, 2026 10:42
@Jordanh1996 Jordanh1996 added the safe to test run tests label Sep 24, 2026
@github-actions github-actions Bot removed safe to test run tests labels Sep 24, 2026
@Jordanh1996 Jordanh1996 changed the title Fix Gradle scan crash and build impact paths per module XRAY-100231 - Keep every Gradle module's dependencies when building the tree Sep 24, 2026
@Jordanh1996 Jordanh1996 changed the title XRAY-100231 - Keep every Gradle module's dependencies when building the tree Keep every Gradle module's dependencies when building the tree Sep 24, 2026
Comment thread src/main/java/com/jfrog/ide/common/gradle/GradleTreeBuilder.java
Comment thread src/test/java/com/jfrog/ide/common/configuration/JfrogCliDriverTest.java Outdated
Comment thread src/main/java/com/jfrog/ide/common/parse/SarifParser.java
Comment thread src/main/java/com/jfrog/ide/common/parse/SarifParser.java
Comment thread src/test/java/com/jfrog/ide/common/gradle/GradleTreeBuilderTest.java Outdated
public void gradleTreeBuilderSharedDependencyTest(String projectPath) throws IOException {
final String COMMONS_TEXT = "org.apache.commons:commons-text:1.9";
final String COMMONS_LANG3 = "org.apache.commons:commons-lang3:3.11";
DepTree depTree = buildGradleDependencyTree(projectPath);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

buildGradleDependencyTree asserts getRootNode().getChildren().size() == 3 (meant for groovy/kotlin). This fixture only passes that because the init script still emits the root project plus moda/modb.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Here the 3 isn't a coincidence: this fixture has exactly the root project plus moda and modb, and each one writes a dependency-tree file. The helper's assertion holds for this fixture for the same reason it does for groovy/kotlin, so I'd leave it.

assertTrue(missing.getScopes().contains("testImplementation"));
}

/**

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

One-row data provider plus javadoc that only returns 'sharedDependency' is noise. A plain @Test fits this fixture.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

The data provider is required. @BeforeMethod setUp(Object[] args) copies the fixture named in args[0], so a plain @Test would reach setUp with no arguments. That's why every test in this class uses one.

Comment thread src/test/java/com/jfrog/ide/common/configuration/JfrogCliDriverTest.java Outdated
@orto17
orto17 self-requested a review September 27, 2026 09:18
Treat a missing results list like an empty one when skipping an unsupported scanner,
and cover the skip with a SARIF fixture holding an SCA run plus unsupported runs with
no, some and missing results. The audit tests look up their SCA finding by file name
instead of counting files, and the shared-dependency test asserts each node and module
exists before reading it.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@github-actions github-actions Bot removed the safe to test run tests label Sep 27, 2026
@github-actions

Copy link
Copy Markdown
Contributor

👍 Frogbot scanned this pull request and did not find any new security issues.


@Jordanh1996
Jordanh1996 merged commit 3f6526e into master Sep 27, 2026
56 of 57 checks passed
@Jordanh1996
Jordanh1996 deleted the fix/XRAY-100231-gradle-node-merge branch September 27, 2026 16:09

This branch was successfully deployed

1 active deployment
frogbot — 9dd03034 Deployed Sep 27, 2026 by Jordanh1996 via scan-pull-request #351
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.

2 participants