Skip to content

Enable windows tests on PRs#15530

Merged
calixtus merged 2 commits intoJabRef:mainfrom
subhramit:subhramit-ci
Apr 13, 2026
Merged

Enable windows tests on PRs#15530
calixtus merged 2 commits intoJabRef:mainfrom
subhramit:subhramit-ci

Conversation

@subhramit
Copy link
Copy Markdown
Member

@subhramit subhramit commented Apr 11, 2026

Triggered by https://github.com/JabRef/jabref/actions/runs/24281108807/job/70903048989

Related issues and pull requests

Closes NA

PR Description

Steps to test

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

Signed-off-by: subhramit <subhramit.bb@live.in>
@qodo-free-for-open-source-projects
Copy link
Copy Markdown
Contributor

Review Summary by Qodo

Enable Windows unit tests for all pull requests

✨ Enhancement

Grey Divider

Walkthroughs

Description
• Enable Windows tests to run on all pull requests
• Remove restriction limiting tests to Gradle Wrapper updates
• Expand CI/CD coverage for Windows platform validation
Diagram
flowchart LR
  PR["Pull Request Event"]
  CONDITION["Conditional Check"]
  TESTS["Windows Tests Execution"]
  PR -- "github.event_name == 'pull_request'" --> CONDITION
  CONDITION -- "Now Always Runs" --> TESTS
Loading

Grey Divider

File Changes

1. .github/workflows/tests-code.yml ⚙️ Configuration changes +1/-1

Expand Windows tests to all pull requests

• Modified the conditional logic for Windows tests job
• Changed from running only on Gradle Wrapper update PRs to running on all PRs
• Removed the startsWith(github.event.pull_request.title, 'Update Gradle Wrapper') condition
• Simplified the condition to just check github.event_name == 'pull_request'

.github/workflows/tests-code.yml


Grey Divider

Qodo Logo

@qodo-free-for-open-source-projects
Copy link
Copy Markdown
Contributor

qodo-free-for-open-source-projects bot commented Apr 11, 2026

Code Review by Qodo

🐞 Bugs (2)   📘 Rule violations (0)   📎 Requirement gaps (0)   🎨 UX Issues (0)
🐞\ ☼ Reliability (1) ➹ Performance (1)

Grey Divider


Remediation recommended

1. Merge queue skips Windows tests 🐞
Description
The workflow is triggered on merge_group, but the tests-windows job is only allowed for
push/workflow_dispatch/pull_request, so merge-queue runs won’t execute Windows unit tests. This
creates different CI coverage between PR runs and merge-queue runs.
Code

.github/workflows/tests-code.yml[285]

+        github.event_name == 'pull_request'
Evidence
The workflow declares merge_group as a trigger, but the tests-windows job’s if-expression does not
include merge_group, so it will be skipped for that event type.

.github/workflows/tests-code.yml[3-10]
.github/workflows/tests-code.yml[278-286]

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

### Issue description
`tests-windows` does not run on `merge_group` events even though the workflow is configured to run on `merge_group`, so merge-queue executions miss Windows unit tests.

### Issue Context
This PR broadens the Windows job to run on all `pull_request` events; to keep CI coverage consistent when using GitHub merge queue, the same Windows job should also execute for `merge_group`.

### Fix Focus Areas
- .github/workflows/tests-code.yml[278-286]
- .github/workflows/tests-code.yml[3-10]

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


2. Duplicate Windows jablib checks 🐞
Description
With tests-windows now running :jablib:check on every pull_request, the tests-windows-file-based
job no longer adds coverage but still allocates a Windows runner and performs a checkout/change
detection. This increases CI time/cost for PRs without providing additional signal.
Code

.github/workflows/tests-code.yml[285]

+        github.event_name == 'pull_request'
Evidence
tests-windows runs gradle :jablib:check on Windows, and tests-windows-file-based also targets
Windows and (conditionally) runs gradle :jablib:check, but the job always starts a Windows runner
even when it does nothing.

.github/workflows/tests-code.yml[278-301]
.github/workflows/tests-code.yml[303-328]

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

### Issue description
After enabling the full Windows `jablib` test job for all PRs, the existing `tests-windows-file-based` job is effectively redundant but still consumes a Windows runner.

### Issue Context
`tests-windows` already executes `:jablib:check` on Windows for PRs. `tests-windows-file-based` also exists to run `:jablib:check` (for a subset of file changes) but always starts a Windows job to determine whether it should run.

### Fix Focus Areas
- .github/workflows/tests-code.yml[303-328]
- .github/workflows/tests-code.yml[278-301]

ⓘ 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

@subhramit subhramit changed the title Enable windows tests on PR Enable windows tests on PRs Apr 11, 2026
@subhramit subhramit requested a review from koppor April 11, 2026 22:57
@subhramit subhramit enabled auto-merge April 12, 2026 23:02
@calixtus calixtus disabled auto-merge April 13, 2026 18:55
@calixtus calixtus merged commit 814147e into JabRef:main Apr 13, 2026
49 of 50 checks passed
calixtus added a commit that referenced this pull request Apr 16, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants