Skip to content

Conversation

robertoschwald
Copy link
Contributor

@robertoschwald robertoschwald commented May 19, 2025

Test-Case for #14753

@matrei
Copy link
Contributor

matrei commented May 19, 2025

Is this test supposed to fail before #14753 is merged?

@robertoschwald
Copy link
Contributor Author

This PRs base branch is apache:issue-11795, the base for PR #14753
@jdaugherty rebased my original PR into a new one I cannot push to.

@matrei
Copy link
Contributor

matrei commented May 19, 2025

OK, but when I run the test added in this PR, with branch 7.0.x checked out, it succeeds.
I would expect it to fail before #14753 is merged.
It makes me wonder if the test validates what it's suppose to validate.
It's likely I'm missing something so I just want to hear if this is expected behavior.

@robertoschwald
Copy link
Contributor Author

The problem with bundle ordering materalized here when adding a PluginAwareResourceBundleMessageSource messageSource definition and sets own basenames to be able to overwrite other bundles keys in a centralized bundle file. Then the plugin basenames were first in the list, not last.
So yes, the test should work against 7.0.0, but also with #14753

@jdaugherty
Copy link
Contributor

Can we add a test for the problem that this fixes? to show it fails in 7 prior to the fix?

@matrei matrei mentioned this pull request May 20, 2025
@jamesfredley jamesfredley moved this to In Progress in Apache Grails Jul 13, 2025
@jamesfredley jamesfredley merged commit e188ad8 into apache:issue-11795 Aug 18, 2025
2 checks passed
@github-project-automation github-project-automation bot moved this from In Progress to Done in Apache Grails Aug 18, 2025
@jdaugherty
Copy link
Contributor

@jamesfredley I don't think this should have been merged. We needed a test case that showed this was broken prior to the fixes

@jamesfredley
Copy link
Contributor

@jdaugherty Agreed. I merged to two newest PRs together as a starting point, since it was disjointed and hard to follow across the set of PRs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

4 participants