paul-rogers commented on PR #12716:
URL: https://github.com/apache/druid/pull/12716#issuecomment-1170255932
Thanks for catching the console issue!
This one is tricky! The `skipTests` flag is used by Surefire. It can be set
via the command line, via a Maven property, or as a plugin property. If you say
`-DskipTests`, then you override anything set in the `pom.xml` files.
We're trying to move to revised ITs, based on Failsafe, which shares code
with Surefire. One of the things it shares is the `skipTests` property. So, if
we set `skipTests`, we disable both the ITs and UTs.
A bit of StackOverflow searching revealed a workaround: define two separate
properties: `skipUTs` for unit tests (Surefire) and `skipITs` for integration
tests (Failsafe). Then, in the `pom.xml`:
```xml
<artifactId>maven-surefire-plugin</artifactId>
<skipTests>${skipUTs}</skipTests>
```
Now, you can still force it with `-DskipTests`, which overrides the above.
With the revised ITs, you will want the fine tuning. The PR that this PR
fixes tried multiple combinations. The present code was the one which _seemed_
to work. This PR points out that the console was broken.
This fix adds `skipTests` back. By so doing, we complicate the process: it
will disable both test systems, just so we can disable the console tests. What
we really want is to decide if those tests are considered UTs or ITs. If UTs,
then the web console `pom.xml` file change is correct.
If we make that change, then we don't need the other changes. Setting
`skipTests` in the `pom.xml` doesn't do anything: we already control Surefilre
as shown above. From the command line, `-DskipTests` should override the
`pom.xml`, since options set on the command line override anything set in the
`pom.xml` file.
At the time of this writing, the build hasn't completed. When I mucked with
this, I ran into several issues. First, one cannot reference an undefined
property. So, just doing the following:
```xml
<skipUTs>${skipTests}</skipUTs>
<skipITs>${skipTests}</skipITs>
```
Will fail the build if `skipTests` is unset.
Also, there is a timing issue. Properties are set at the start of the build.
Profiles come later. So, a profile cannot set a property which is referenced in
the properties section (IIRC.). This, proposed change will probably not work:
first we set the properties based on `skipTests=false`, then we later change
`skipTests=true`, but Maven isn't smart enough to recompute `skipITs`, etc. I
think I recall something about early- vs. late-binding properties. So, I'll
caveat the above comment by saying I encountered the issue, went the route in
the current code, and didn't dig into the binding reference.
Bottom line:
* The web console fix is good. Thanks for catching that.
* The `pom.xml` changes shouldn't be needed.
* Setting `-DskipTest` on the command line should work. If it doesn't, I'm
missing something (which is likely, Maven is hard to understand.)
* Setting `-P skip-tests` on the command line is what Travis does and seems
to be the "official" way to skip tests.
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
