Adapt legacyui tests to TRUNK-6558 (JobRunr scheduler) and AddressSupport singleton - #260
Open
wikumChamith wants to merge 3 commits into
Open
Adapt legacyui tests to TRUNK-6558 (JobRunr scheduler) and AddressSupport singleton#260wikumChamith wants to merge 3 commits into
wikumChamith wants to merge 3 commits into
Conversation
…port singleton 1. Reset AddressSupport singleton state in ShortPatientFormValidatorTest PersonAddressValidator was changed in core (openmrs/openmrs-core#5839) to read the address template via the AddressSupport singleton instead of LocationService.getAddressTemplate(). The singleton caches the template across tests — when AddressSupportTest loads personAddressValidatorTestDataset.xml (which sets <requiredElements><string>address1</string></requiredElements>) into the global property, every subsequent test in the same JVM inherits address1 as a required field, even though the DB-level GP is rolled back at end-of-test. Fix: a @beforeeach in ShortPatientFormValidatorTest now sets layout.address.format via AdministrationService.setGlobalProperty(...) to a template with empty <requiredElements/>. This goes through the AdminService listener chain and refreshes the cached singleton state to a known-good baseline before each test. 2. Disable scheduler tests broken by JobRunr migration openmrs/openmrs-core#5893 replaced TimerSchedulerServiceImpl with JobRunrSchedulerService. The new scheduleTask(TaskDefinition) returns null and never populates TaskDefinition.taskInstance — the legacy in-memory pointer that the disabled tests rely on. TaskDefinition/Task are now @deprecated; "is currently executing" is observable only through the new SchedulerService.getTask(uuid).getState() == TaskState.PROCESSING API. Four tests are marked @disabled with comments pointing at TRUNK-6558: - SchedulerFormControllerTest.onSubmit_shouldRescheduleACurrentlyScheduledTask - SchedulerFormControllerTest.onSubmit_shouldNotRescheduleAnExecutingTask - TaskHelperTest.waitUntilTaskIsExecuting_shouldWaitUntilTaskIsExecuting - TaskHelperTest.waitUntilTaskIsExecuting_shouldRaiseATimeoutExceptionWhenTheTimeoutIsExceeded (cherry picked from commit d0d3232)
wikumChamith
force-pushed
the
fix-TRUNK-6558
branch
from
April 27, 2026 11:22
d0d3232 to
6b39c3f
Compare
Member
|
Did you get a chance to look at the failing builds? |
Member
Author
Yeah I am looking at them. I am only seeing them on GHA |
…port singleton ▎ Fix: a @beforeeach in ShortPatientFormValidatorTest and ShortPatientFormControllerTest sets layout.address.format to a template with empty ▎ <requiredElements/> and directly invokes AddressSupport.getInstance().globalPropertyChanged(...). The direct call is needed because in CI the ▎ singleton's listener can be registered against a stale Spring context, in which case setGlobalProperty notifies a different listener list and the ▎ cached template stays polluted.
Member
Author
Fixed it :) |
…port singleton Two openmrs-core 3.0.0-SNAPSHOT changes broke the legacyui suite: - TRUNK-6558 swapped the timer-based SchedulerService for JobRunr. TaskDefinition.taskInstance is no longer populated, so anything reading task.getTaskInstance().isExecuting() is broken. Live state is now on SchedulerService.getTask(uuid).getState(). - LUI-199 moved address-template reads behind the AddressSupport singleton, which caches per-JVM and inherits dirty state from earlier tests in the run. Changes - SchedulerFormController — fix the "don't reschedule a running task" guard (was always true under JobRunr, so the controller silently rescheduled in-flight jobs). Now uses getTask(uuid).getState() != PROCESSING. - ShortPatientFormValidatorTest / ShortPatientFormControllerTest — @beforeeach resets the AddressSupport singleton: writes a clean layout.address.format GP and calls AddressSupport.getInstance().globalPropertyChanged(...) directly (the listener can be registered against a stale Spring context on CI). - TaskHelper.waitUntilTaskIsExecuting — polls SchedulerService.getTask(uuid).getState() instead of taskInstance. Wait budget bumped 2s→60s to match JobRunr's poll cadence. - TaskHelperTest.xml — repeat_interval 3600→0 so the scheduled job is a one-off and observable via getTask(task.uuid). - TaskHelperTest — both wait tests re-enabled. - SchedulerFormControllerTest — the two reschedule tests stay @disabled. Verifying them needs JobRunr to actually persist + run jobs across a test transaction, which BaseModuleWebContextSensitiveTest's rollback-per-test base doesn't support. Equivalent coverage lives in core's SchedulerServiceIT.
Member
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
|
Warning Rate limit exceeded
To keep reviews running without waiting, you can enable usage-based add-on for your organization. This allows additional reviews beyond the hourly cap. Account admins can enable it under billing. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (7)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Review rate limit: 0/1 reviews remaining, refill in 5 minutes and 36 seconds.Comment |
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Two openmrs-core 3.0.0-SNAPSHOT changes broke the legacyui suite:
task.getTaskInstance().isExecuting() is broken. Live state is now on SchedulerService.getTask(uuid).getState().
run.
Changes
in-flight jobs). Now uses getTask(uuid).getState() != PROCESSING.
@BeforeEachresets the AddressSupport singleton: writes a cleanlayout.address.format GP and calls AddressSupport.getInstance().globalPropertyChanged(...) directly (the listener can be registered against a stale
Spring context on CI).
JobRunr's poll cadence.
@Disabled. Verifying them needs JobRunr to actually persist + run jobs across a testtransaction, which BaseModuleWebContextSensitiveTest's rollback-per-test base doesn't support. Equivalent coverage lives in core's
SchedulerServiceIT.