Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update MacroRecorderPostprocessor tests #307

Merged
merged 3 commits into from
Jul 22, 2024
Merged

Conversation

hinerm
Copy link
Member

@hinerm hinerm commented Jul 19, 2024

This fixes #305 for me

The LegacyService has a setter for its IJ1Helper. Tracking it separately
in the DefaultLegacyHooks creates an opportunity for skew between its
helper and the LegacyService's helper. The LegacyService has an accessor
for its IJ1Helper instance so it is not necessary to cache it in the
DefaultLegacyHooks.

See #305
In my Windows environment (command line and IntelliJ) the
MacroRecorderPostprocessor was not being registered as a postprocessor.
Doing this seems necessary for the test to pass.
@hinerm hinerm requested a review from ctrueden July 19, 2024 18:54
@hinerm
Copy link
Member Author

hinerm commented Jul 19, 2024

@ctrueden can you check if this works on your systems?

Copy link
Member

@ctrueden ctrueden left a comment

Choose a reason for hiding this comment

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

All tests pass with this branch on my Linux system!

I see that all references to the helper variable have changed to access it dynamically from the LegacyService every time. But there is still a null check in at least one spot. Is the idea that the IJ1Helper can go from null to non-null or vice versa? If so, what if it happens in between the null check and the next dynamic access?

src/main/java/net/imagej/legacy/DefaultLegacyHooks.java Outdated Show resolved Hide resolved
In case we want to do null checking in the future.
@hinerm hinerm merged commit 786a58b into master Jul 22, 2024
1 check passed
@hinerm hinerm deleted the macro-recorder-postprocessor branch July 22, 2024 15:24
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.

Building locally hangs on Windows
2 participants