Enforce Quality Gates for Java and Python sub projects on CI by ndegwamartin · Pull Request #1543 · google/fhir-data-pipes

ndegwamartin · 2025-12-19T15:53:08Z

Description of what I changed

Resolves #1541

E2E test

TESTED:

Please replace this with a description of how you tested your PR beyond the
automated e2e/unit tests.

Checklist: I completed these to help reviewers :)

I have read and will follow the
review process.
I am familiar with Google Style Guides for the language I have coded in.

No? Please take some time and review
Java and
Python style guides.
My IDE is configured to follow the Google
code styles.

No? Unsure? ->
configure your IDE.
I have added tests to cover my changes. (If you refactored existing
code that was well tested you do not have to add tests)
I ran mvn clean package right before creating this pull request and
added all formatting changes to my commit.
If I made any Python code changes, I ran black ., pylint . and
pyright . right before creating this pull request and added all
formatting changes to my commit.
All new and existing tests passed.
My pull request is based on the latest changes of the master branch.

No? Unsure? -> execute command git pull --rebase upstream master

codecov-commenter · 2025-12-22T04:47:26Z

Codecov Report

❌ Patch coverage is 73.17073% with 11 lines in your changes missing coverage. Please review.
✅ Project coverage is 46.75%. Comparing base (8434457) to head (9a97804).

Files with missing lines	Patch %	Lines
...unsen/avro/converters/DefinitionToAvroVisitor.java	75.00%	6 Missing ⚠️
...ava/com/google/fhir/analytics/PipelineManager.java	70.00%	3 Missing ⚠️
...m/cerner/bunsen/avro/converters/NoOpConverter.java	0.00%	1 Missing ⚠️
...ava/com/google/fhir/analytics/DwhFilesManager.java	83.33%	0 Missing and 1 partial ⚠️

Additional details and impacted files

@@             Coverage Diff              @@
##             master    #1543      +/-   ##
============================================
+ Coverage     46.61%   46.75%   +0.13%     
- Complexity      678      683       +5     
============================================
  Files            90       90              
  Lines          5886     5893       +7     
  Branches        834      834              
============================================
+ Hits           2744     2755      +11     
+ Misses         2827     2822       -5     
- Partials        315      316       +1

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

…pylint error W1203

bashir2

Thanks @ndegwamartin for the changes; just a few minor suggestions/questions.

bunsen/bunsen-avro/src/main/java/com/cerner/bunsen/avro/converters/DefinitionToAvroVisitor.java

bunsen/bunsen-avro/src/test/java/com/cerner/bunsen/avro/R4AvroConverterUsCoreTest.java

pipelines/batch/src/main/java/com/google/fhir/analytics/HapiRowDescriptor.java

pipelines/controller-cli/src/main.py

pipelines/controller/src/main/java/com/google/fhir/analytics/PipelineManager.java

.github/workflows/python-lint.yml

bashir2

Thanks @ndegwamartin please feel free to merge after addressing the remaining comments below.

bunsen/bunsen-avro/src/main/java/com/cerner/bunsen/avro/converters/DefinitionToAvroVisitor.java

pipelines/controller-cli/src/main.py

pipelines/controller/src/main/java/com/google/fhir/analytics/PipelineManager.java

- Reverts lazy logging for main.py - Disables lazy logging check in pylint configuration - Refactors get current time to use static helper method - Adds unit tests for PipelineManager class

-Fix pylint warning

pipelines/controller/src/main/java/com/google/fhir/analytics/DwhFilesManager.java

pipelines/controller/src/test/java/com/google/fhir/analytics/PipelineManagerTest.java

requirements-dev.txt

ndegwamartin · 2026-01-13T13:48:10Z

/gcbrun

bashir2

Just some minor unit-test/architecture related suggestion.

bashir2 · 2026-01-16T06:51:51Z

pipelines/controller/src/test/java/com/google/fhir/analytics/PipelineManagerTest.java

+    MeterRegistry meterRegistry = mock(MeterRegistry.class);
+    pipelineManager = new PipelineManager();
+    setField("dataProperties", dataProperties);
+    setField("dwhFilesManager", dwhFilesManager);


I am guessing that you are forced to use reflection for these fields because we use @Autowired on fields, correct? So I think a better solution is to avoid that pattern and instead use constructor injection instead, i.e., we will have a constructor on which we have an @Autowired annotation. Then we can use the constructor directly here with mock objects. If you do that pattern then great, if you feel that is too much work, please add a TODO to switch to that pattern at some point.

bashir2 · 2026-01-16T06:53:32Z

pipelines/controller/src/test/java/com/google/fhir/analytics/PipelineManagerTest.java

+  public void testIncrementalModeTriggeredAtRightTime() throws Exception {
+    // Mock current time to be after next scheduled time
+    LocalDateTime currentTime = lastRunEndTimestamp.plusMinutes(2); // 2 minutes after lastRunEnd
+    try (MockedStatic<DwhFilesManager> mockedDwh = mockStatic(DwhFilesManager.class)) {


If we inject DwhFilesManager as well then we can change getCurrentTime to be non-static making it easier to mock, but again this is just a suggestion.

ndegwamartin added 4 commits December 19, 2025 17:36

Fixes ErrorProne warnings on master

f05e9c0

Enables CI Gate check for Java checks

5573874

Updates Maven configuration for ErrorProne

9f4e8c1

Sets up Github actions workflow for python code quality

82f88da

ndegwamartin added 3 commits December 22, 2025 08:13

Refactors f-string logger interpolations to use lazy % format, fixes …

0dea1b1

…pylint error W1203

Removes Python static checks for synthea sub-projects

2699774

Fixes Pylint error W1405: Inconsistent-quotes

793467f

ndegwamartin marked this pull request as ready for review December 22, 2025 05:34

ndegwamartin requested a review from bashir2 December 22, 2025 05:34

ndegwamartin added 2 commits December 22, 2025 19:01

Updates Python Code Quality workflow configuration

ac552a2

Updates Python lint workflow to include pip dev dependencies

e9aff5f

bashir2 reviewed Dec 23, 2025

View reviewed changes

bashir2 approved these changes Dec 23, 2025

View reviewed changes

Address PR Feedback

5991c60

- Reverts lazy logging for main.py - Disables lazy logging check in pylint configuration - Refactors get current time to use static helper method - Adds unit tests for PipelineManager class

ndegwamartin force-pushed the enable-ci-gates branch from 716b7e9 to 5991c60 Compare December 29, 2025 20:18

ndegwamartin added 2 commits December 30, 2025 17:02

Pin python dependencies versions

bf16f84

-Fix pylint warning

Clean up

6822c79

bashir2 reviewed Jan 13, 2026

View reviewed changes

Address PR Feedback II

a1cad1c

bashir2 approved these changes Jan 16, 2026

View reviewed changes

ndegwamartin and others added 2 commits January 29, 2026 16:37

Refactors PipelineManager to constructor injection for testability

097c91e

Merge branch 'master' into enable-ci-gates

9a97804

Conversation

ndegwamartin commented Dec 19, 2025

Description of what I changed

E2E test

Checklist: I completed these to help reviewers :)

Uh oh!

codecov-commenter commented Dec 22, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

bashir2 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

bashir2 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ndegwamartin commented Jan 13, 2026

Uh oh!

bashir2 left a comment

Choose a reason for hiding this comment

Uh oh!

bashir2 Jan 16, 2026

Choose a reason for hiding this comment

Uh oh!

bashir2 Jan 16, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Comments

codecov-commenter commented Dec 22, 2025 •

edited

Loading