jdaugherty commented on PR #15731: URL: https://github.com/apache/grails-core/pull/15731#issuecomment-4708727576
> > > @jdaugherty please explain why? We were missing a ton of issues and breaking changes due to the missing tests on h7? This is working and tests are not production code for end apps. This is DRY > > > I think we should run the same tests against both. > > > > > > I previously have explained this on multiple PRs. The issue is it complicates the setup, running locally, and troubleshooting dependency resolution. Any missing coverage we should add tests for, you have already identified the gaps so it should be easy. > > I think we have to switch hibernate modules for functional tests. I don't think it is scalable to duplicate all test apps. It will create a lot of duplication and a heavy maintenance burden. Can we create shell scripts or other solution to easily run both/all versions locally? We don't need to duplicate the functional apps themself, but rather the test coverage in the TCK or the example apps to ensure proper coverage. The problem with having switches for functionality is the build tools behave differently based on those switches. This means: 1. Gradle caching is broken when dependencies change (this is huge and made worse by the mono project) 2. Breaks debugging / troubleshooting since switches have to be populated through every tool (from the CLI to intellij) 3. Tools like IntelliJ can't easily pass parameters so they will use what ever is the default. If hibernate5 is the default now, and there's a problem with 7 it's not easy to debug. Once we switch, it will be the opposite. 4. Test coverage reports don't make it clear which version was used. This means anytime we look at code coverage or other tools we can't determine which one was the problem. We effectively hide behavior behind a switch and Gradle is not designed for that. I'm a strong no for this reason. I believe we can add test coverage, but tests in a functional app for UrlMappings don't need hibernate and shouldn't need to be cloned. We should clone the tests that are relevant or add scenarios for them. We need to do this anyway to prevent regressions. I realize that substitution is a short cut to getting this, but I view it as a major blocker for regular development & proper project hygiene. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
