jdaugherty commented on code in PR #16472:
URL: https://github.com/apache/grails-core/pull/16472#discussion_r4169290672
##########
grails-testing-support-core/src/main/groovy/org/grails/compiler/injection/testing/IntegrationTestAstTransformation.groovy:
##########
@@ -131,6 +132,29 @@ class IntegrationTestAstTransformation implements
ASTTransformation, TransformWi
}
+ /**
+ * The classpath the spec is compiled with: the compiler configuration's
entries and those of the class loaders it
+ * compiles against. It carries the application's main classes directory
wherever the build puts it. Dynamic, as
+ * static flow typing would hold the loop's class loader to the first
one's type and fail on its parent.
+ */
+ @CompileDynamic
+ private static Collection<File> compileClasspath(SourceUnit source) {
Review Comment:
Nothing tests `compileClasspath`. `MainClassFinderSpec` passes the classpath
in directly, so no test checks what the transform hands it, and Codecov reports
the new lines in this file as uncovered. Could a spec compile an `@Integration`
class the way a build does? That means a source file inside a project
directory, compiled against an application class in a classes directory that
isn't under `build/`, and then checking that the compiled class has
`@ContextConfiguration(classes = Application)`. It would also be the natural
place to cover the root-level layout from the other comment.
##########
grails-gradle/model/src/main/groovy/org/grails/io/support/MainClassFinder.groovy:
##########
@@ -85,6 +100,13 @@ class MainClassFinder {
}
if (rootDir) {
+ File projectDir = rootDir.canonicalFile
+ for (File entry in classpath) {
+ if (entry.isDirectory() &&
findRootDirectory(entry)?.canonicalFile == projectDir) {
Review Comment:
This only accepts a classes directory inside the spec's own project, so a
build directory at the root or outside the checkout still finds nothing. The
`bootRun` provider handles those layouts (the `../` feature), but
`@Integration` doesn't.
I moved `enable-mvc-check`'s build directory to `<root>/build-out/<project>`
from an init script, which is the
`rootProject.layout.buildDirectory.dir(project.name)` layout, and left no
`build/` in the project. The spec gets no `@ContextConfiguration`, and all 7
`integrationTest` tests fail with `No baseUrl set`. Calling `searchMainClass`
directly with the classes from this head gives the same answer. It returns null
for a root-level build directory and for one outside the checkout, and finds
`example.Application` when the build directory is inside the project.
The compile's own output directory could decide this instead of where the
directory sits. Gradle sets `CompilerConfiguration.targetDirectory` to the
task's destination (`<build>/classes/groovy/integrationTest`), and the main
classes directory is next to it under `<build>/classes`, wherever the build
directory is. A plugin subproject's classes directory is under that
subproject's own build directory, so it is still left out. As a throwaway
probe, I passed `source.configuration.targetDirectory` from the transform and
also accepted a classpath directory that shares the target's `<build>/classes`
parent. With that change the root-level layout passes all 7 tests. The current
check could stay as the fallback for a compile with no target directory.
While some layouts find nothing, this also affects the build cache. Whether
the application class is found isn't part of `compileIntegrationTestGroovy`'s
cache key, because Gradle compares the compile classpath by its contents, not
by where its directories are. After the root-level run, a run with the build
directory inside the project restored the spec from the local build cache
without `@ContextConfiguration`, and all 7 tests failed. The same run with
`--no-build-cache` and a clean output directory passed. Once every layout finds
the class, the compiled output is the same for all of them and this goes away.
##########
grails-gradle/model/src/main/groovy/grails/util/BuildSettings.groovy:
##########
@@ -335,6 +337,24 @@ class BuildSettings {
BASE_DIR = System.getProperty(APP_BASE_DIR) ? new
File(System.getProperty(APP_BASE_DIR)) :
(IOUtils.findApplicationDirectoryFile() ?: new File('.'))
GRAILS_APP_DIR_PRESENT = new File(BASE_DIR, 'grails-app').exists() ||
new File(BASE_DIR, 'Application.groovy').exists()
TARGET_DIR = new File(BASE_DIR,
System.getProperty('project.target.dir', 'build'))
- RESOURCES_DIR = !GRAILS_APP_DIR_PRESENT ? null :
(System.getProperty(PROJECT_RESOURCES_DIR) ? new
File(System.getProperty(PROJECT_RESOURCES_DIR)) : new File(TARGET_DIR,
'resources/main'))
+ RESOURCES_DIR = !GRAILS_APP_DIR_PRESENT ? null :
resourcesDir(System.getProperty(PROJECT_RESOURCES_DIR), BASE_DIR, TARGET_DIR)
+ }
+
+ /**
+ * {@link #RESOURCES_DIR}: a {@link #PROJECT_RESOURCES_DIR} that is
relative, as the Gradle plugin passes it, joined
+ * to the application directory rather than the working directory, which a
build can move; one that is absolute
+ * as it is; and without one, {@code resources/main} in the target
directory.
+ *
+ * @param fromSystem the {@link #PROJECT_RESOURCES_DIR} system property,
or null
+ * @param baseDir the application directory
+ * @param targetDir the target directory
+ * @return the resources directory
+ */
+ static File resourcesDir(String fromSystem, File baseDir, File targetDir) {
Review Comment:
Minor: `resourcesDir` is public, so `BuildSettings` gains a public static
method that only the first feature in `BuildSettingsSpec` calls. Could it be
private, with that feature's absolute case moved into the forked-JVM feature?
That feature already checks `RESOURCES_DIR` itself, which is what the
application reads.
--
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]
