Skip to content

Add names to output#15102

Merged
koppor merged 6 commits intomainfrom
add-test-names
Feb 13, 2026
Merged

Add names to output#15102
koppor merged 6 commits intomainfrom
add-test-names

Conversation

@koppor
Copy link
Member

@koppor koppor commented Feb 12, 2026

Follow-up to #13535

Test stopped working today

Checklist

  • I own the copyright of the code submitted and I license it under the MIT license
  • I manually tested my changes in running JabRef (always required)
  • I added JUnit tests for changes (if applicable)
  • [/] I added screenshots in the PR description (if change is visible to the user)
  • [/] I added a screenshot in the PR description showing a library with a single entry with me as author and as title the issue number
  • [/] I described the change in CHANGELOG.md in a way that can be understood by the average user (if change is visible to the user)
  • [/] I checked the user documentation for up to dateness and submitted a pull request to our user documentation repository

@testlens-app
Copy link

testlens-app bot commented Feb 12, 2026

✅ All tests passed ✅

🏷️ Commit: f55bcde
▶️ Tests: 11186 executed
⚪️ Checks: 66/66 completed


Learn more about TestLens at testlens.app.

@koppor koppor marked this pull request as ready for review February 13, 2026 05:52
@koppor koppor added the automerge PR is tagged with that label will be merged if workflows are green label Feb 13, 2026
@koppor koppor enabled auto-merge February 13, 2026 05:52
@qodo-free-for-open-source-projects
Copy link
Contributor

Review Summary by Qodo

Add names to parameterized tests and improve debugging

🧪 Tests 🐞 Bug fix

Grey Divider

Walkthroughs

Description
• Add test names to parameterized test output for better visibility
• Add trace logging to LinkedFileTransferHelper for debugging
• Fix typo in LinkedFile comment (followin → following)
• Disable non-working test case and test-logger plugin
Diagram
flowchart LR
  A["Test Parameters"] -->|Add name parameter| B["Parameterized Test Output"]
  C["LinkedFileTransferHelper"] -->|Add trace logs| D["Debug Information"]
  E["Code Quality"] -->|Fix typos| F["Documentation"]
  G["Build Config"] -->|Disable test-logger| H["Simplified Build"]
Loading

Grey Divider

File Changes

1. jablib/src/test/java/org/jabref/logic/externalfiles/LinkedFileTransferHelperTest.java 🧪 Tests +16/-12

Add names to parameterized test cases

• Add testName parameter to parameterized test with @ParameterizedTest(name = "{0}")
• Add descriptive names to all test case Arguments.of() calls
• Comment out one failing test case with note about breakage date
• Extract expected entry to variable for clarity
• Remove IOException from check() method signature

jablib/src/test/java/org/jabref/logic/externalfiles/LinkedFileTransferHelperTest.java


2. jablib/src/main/java/org/jabref/logic/externalfiles/LinkedFileTransferHelper.java ✨ Enhancement +3/-1

Add trace logging for debugging file transfer

• Add trace log when file is detected as absolute path
• Add trace log when target context lacks primary directory
• Add trace log when source file does not exist

jablib/src/main/java/org/jabref/logic/externalfiles/LinkedFileTransferHelper.java


3. jablib/src/main/java/org/jabref/model/entry/LinkedFile.java 🐞 Bug fix +1/-1

Fix typo in comment

• Fix typo in comment: "followin" → "following"

jablib/src/main/java/org/jabref/model/entry/LinkedFile.java


View more (2)
4. build-logic/src/main/kotlin/org.jabref.gradle.feature.test.gradle.kts ⚙️ Configuration changes +3/-1

Disable test-logger plugin

• Comment out test-logger plugin declaration
• Comment out entire testlogger configuration block

build-logic/src/main/kotlin/org.jabref.gradle.feature.test.gradle.kts


5. jablib/src/test/resources/tinylog-test.properties ⚙️ Configuration changes +2/-0

Enable trace logging for LinkedFileTransferHelper

• Add trace level logging for LinkedFileTransferHelper class

jablib/src/test/resources/tinylog-test.properties


Grey Divider

Qodo Logo

@JabRef JabRef deleted a comment from github-actions bot Feb 13, 2026
@qodo-free-for-open-source-projects
Copy link
Contributor

Code Review by Qodo

🐞 Bugs (1) 📘 Rule violations (3) 📎 Requirement gaps (0)

Grey Divider


Action required

1. Commented testlogger {} block 📘 Rule violation ⛯ Reliability
Description
A full testlogger { ... } configuration block is wrapped in a block comment, leaving commented-out
code in the build logic. This reduces maintainability and violates the rule requiring commented-out
code to be removed.
Code

build-logic/src/main/kotlin/org.jabref.gradle.feature.test.gradle.kts[R27-30]

+/*
testlogger {
    // See https://github.com/radarsh/gradle-test-logger-plugin#configuration for configuration options
Evidence
The compliance checklist requires removing commented-out code. The build script now contains an
entire configuration block inside /* ... */.

AGENTS.md
build-logic/src/main/kotlin/org.jabref.gradle.feature.test.gradle.kts[27-39]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
A full `testlogger {}` block is present as commented-out code.

## Issue Context
Keeping commented-out blocks in build logic violates the project's requirement to remove commented-out code and keep changes reviewable.

## Fix Focus Areas
- build-logic/src/main/kotlin/org.jabref.gradle.feature.test.gradle.kts[27-39]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


2. Trace logs expose file paths 📘 Rule violation ⛨ Security
Description
New LOGGER.trace(...) statements log Path values (including absolute paths), which may contain
sensitive personal information (e.g., usernames in home directories). This violates the secure
logging requirement to avoid logging sensitive/personal data at any level.
Code

jablib/src/main/java/org/jabref/logic/externalfiles/LinkedFileTransferHelper.java[R62-67]

            Path linkedFileAsPath = Path.of(linkedFile.getLink());
            if (linkedFileAsPath.isAbsolute()) {
+                LOGGER.trace("File {} is an absolute path, skipping", linkedFileAsPath);
                // In case the file is an absolute path, there is no need to adjust anything
                // [impl->req~logic.externalfiles.file-transfer.reachable-no-copy~1]
                linkedFiles.add(linkedFile);
Evidence
Secure logging rules prohibit logging sensitive/personal information; absolute file paths can reveal
user-specific data. The added trace logs include linkedFileAsPath directly in the log message,
including the case where it is absolute.

Rule 5: Generic: Secure Logging Practices
jablib/src/main/java/org/jabref/logic/externalfiles/LinkedFileTransferHelper.java[62-76]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Trace logs print file system paths which can include personal/sensitive information.

## Issue Context
Secure logging standards require that logs do not include sensitive data (including personal user information) at any log level.

## Fix Focus Areas
- jablib/src/main/java/org/jabref/logic/externalfiles/LinkedFileTransferHelper.java[62-76]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


3. Commented-out test case 📘 Rule violation ⛯ Reliability
Description
A parameterized test case is disabled by commenting out an Arguments.of(...) block, weakening test
coverage and leaving commented-out code in the test source. This violates the requirements to remove
commented-out code and to not weaken tests/assertions when behavior is being changed or fixed.
Code

jablib/src/test/java/org/jabref/logic/externalfiles/LinkedFileTransferHelperTest.java[R122-127]

+                /* Stopped working 2026-02-12, not sure why
                Arguments.of(
+                        "File in main file directory linked as is",
                        FileTestConfigurationBuilder
                                .fileTestConfiguration()
                                .mainFileDir("main-file-dir")
Evidence
The compliance checklist disallows commented-out code, and also requires keeping tests
strong/deterministic without weakening coverage. The PR comments out an Arguments.of(...) scenario
entirely, effectively disabling that test case.

AGENTS.md
AGENTS.md
jablib/src/test/java/org/jabref/logic/externalfiles/LinkedFileTransferHelperTest.java[122-141]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
A parameterized test scenario is disabled by commenting it out, weakening test coverage.

## Issue Context
The compliance rules require removing commented-out code and maintaining strong/deterministic tests for behavior changes.

## Fix Focus Areas
- jablib/src/test/java/org/jabref/logic/externalfiles/LinkedFileTransferHelperTest.java[122-141]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools



Advisory comments

4. Very verbose test logging 🐞 Bug ✧ Quality
Description
Tests configure LinkedFileTransferHelper at TRACE level, which also enables its DEBUG logs; this
can produce noisy CI output (including file paths) and make failures harder to scan if left enabled
permanently.
Code

jablib/src/test/resources/tinylog-test.properties[R4-8]

#level@org.jabref.model.entry.BibEntry = debug
#level@org.jabref.logic.importer.fetcher.ResearchGate = trace
#level@org.jabref.logic.importer.fetcher.DoiFetcher = trace
+
+level@org.jabref.logic.externalfiles.LinkedFileTransferHelper = trace
Evidence
The test tinylog configuration enables TRACE for a class that logs multiple DEBUG messages including
file locations. If the test logging file is used broadly across the test suite, this can increase
log volume.

jablib/src/test/resources/tinylog-test.properties[1-8]
jablib/src/main/java/org/jabref/logic/externalfiles/LinkedFileTransferHelper.java[50-57]
jablib/src/main/java/org/jabref/logic/externalfiles/LinkedFileTransferHelper.java[132-145]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
Test logging enables TRACE for LinkedFileTransferHelper, which can significantly increase console output.

### Issue Context
The helper logs various DEBUG messages (including file paths). TRACE typically should be temporary or narrowly scoped.

### Fix Focus Areas
- jablib/src/test/resources/tinylog-test.properties[1-8]
- jablib/src/main/java/org/jabref/logic/externalfiles/LinkedFileTransferHelper.java[50-57]
- jablib/src/main/java/org/jabref/logic/externalfiles/LinkedFileTransferHelper.java[132-145]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

ⓘ The new review experience is currently in Beta. Learn more

Grey Divider

Qodo Logo

Comment on lines +27 to 30
/*
testlogger {
// See https://github.com/radarsh/gradle-test-logger-plugin#configuration for configuration options

Choose a reason for hiding this comment

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

Action required

1. Commented testlogger {} block 📘 Rule violation ⛯ Reliability

A full testlogger { ... } configuration block is wrapped in a block comment, leaving commented-out
code in the build logic. This reduces maintainability and violates the rule requiring commented-out
code to be removed.
Agent Prompt
## Issue description
A full `testlogger {}` block is present as commented-out code.

## Issue Context
Keeping commented-out blocks in build logic violates the project's requirement to remove commented-out code and keep changes reviewable.

## Fix Focus Areas
- build-logic/src/main/kotlin/org.jabref.gradle.feature.test.gradle.kts[27-39]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Comment on lines 62 to 67
Path linkedFileAsPath = Path.of(linkedFile.getLink());
if (linkedFileAsPath.isAbsolute()) {
LOGGER.trace("File {} is an absolute path, skipping", linkedFileAsPath);
// In case the file is an absolute path, there is no need to adjust anything
// [impl->req~logic.externalfiles.file-transfer.reachable-no-copy~1]
linkedFiles.add(linkedFile);

Choose a reason for hiding this comment

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

Action required

2. Trace logs expose file paths 📘 Rule violation ⛨ Security

New LOGGER.trace(...) statements log Path values (including absolute paths), which may contain
sensitive personal information (e.g., usernames in home directories). This violates the secure
logging requirement to avoid logging sensitive/personal data at any level.
Agent Prompt
## Issue description
Trace logs print file system paths which can include personal/sensitive information.

## Issue Context
Secure logging standards require that logs do not include sensitive data (including personal user information) at any log level.

## Fix Focus Areas
- jablib/src/main/java/org/jabref/logic/externalfiles/LinkedFileTransferHelper.java[62-76]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Comment on lines +122 to 127
/* Stopped working 2026-02-12, not sure why
Arguments.of(
"File in main file directory linked as is",
FileTestConfigurationBuilder
.fileTestConfiguration()
.mainFileDir("main-file-dir")

Choose a reason for hiding this comment

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

Action required

3. Commented-out test case 📘 Rule violation ⛯ Reliability

A parameterized test case is disabled by commenting out an Arguments.of(...) block, weakening test
coverage and leaving commented-out code in the test source. This violates the requirements to remove
commented-out code and to not weaken tests/assertions when behavior is being changed or fixed.
Agent Prompt
## Issue description
A parameterized test scenario is disabled by commenting it out, weakening test coverage.

## Issue Context
The compliance rules require removing commented-out code and maintaining strong/deterministic tests for behavior changes.

## Fix Focus Areas
- jablib/src/test/java/org/jabref/logic/externalfiles/LinkedFileTransferHelperTest.java[122-141]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

@koppor koppor added this pull request to the merge queue Feb 13, 2026
@github-actions github-actions bot added the status: to-be-merged PRs which are accepted and should go into the merge-queue. label Feb 13, 2026
Merged via the queue into main with commit 6a39214 Feb 13, 2026
61 of 67 checks passed
@koppor koppor deleted the add-test-names branch February 13, 2026 06:32
Siedlerchr added a commit to faneeshh/jabref that referenced this pull request Feb 14, 2026
* upstream/main:
  Add names to output (JabRef#15102)
  Try other token
  Be less strict on ourselves
  Very short self unassigment text
  Fix review check
  Fix trigger
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

automerge PR is tagged with that label will be merged if workflows are green status: to-be-merged PRs which are accepted and should go into the merge-queue.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants