You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@lucene.apache.org by GitBox <gi...@apache.org> on 2022/05/20 09:53:49 UTC
[GitHub] [lucene] dweiss opened a new pull request, #909: LUCENE-10370: pass proper classpath/module arguments for forking jvms from within tests
dweiss opened a new pull request, #909:
URL: https://github.com/apache/lucene/pull/909
This is putting some lipstick on the pig. Forking those JVMs is really an awkward thing to do. I understand some tests are specifically testing JVM crashes, etc., but, uh, those process builders/ pipes are ugly.
Anyway. I tracked down java classpath references in a few test classes and replaced them with a single method on LuceneTestCase that passes proper classpath/ module path/ opens, consistent with those the test classes are running with. I ran nightly tests of the core+replication and all seems fine.
The downside is that folks will no longer be able to run those tests from within IDEs, but I think it's an acceptable tradeoff given how complex those jvm arguments can get (some things on module path, some on classpath, etc.).
--
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: issues-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene] dweiss commented on pull request #909: LUCENE-10370: pass proper classpath/module arguments for forking jvms from within tests
Posted by GitBox <gi...@apache.org>.
dweiss commented on PR #909:
URL: https://github.com/apache/lucene/pull/909#issuecomment-1134373557
No. I think it's the parent path that is missing here.
--
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: issues-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene] dweiss commented on pull request #909: LUCENE-10370: pass proper classpath/module arguments for forking jvms from within tests
Posted by GitBox <gi...@apache.org>.
dweiss commented on PR #909:
URL: https://github.com/apache/lucene/pull/909#issuecomment-1134375202
I know why you're getting it. clean executes after configuration and wipes the temporary task directory for test. We'll have to recreate it properly. I'll commit a fix soon.
--
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: issues-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene] romseygeek commented on pull request #909: LUCENE-10370: pass proper classpath/module arguments for forking jvms from within tests
Posted by GitBox <gi...@apache.org>.
romseygeek commented on PR #909:
URL: https://github.com/apache/lucene/pull/909#issuecomment-1134360770
Hiya, this is making the elasticsearch CI cross; all builds are failing with this message:
```
* Where:
08:09:37 Script '/var/lib/jenkins/workspace/apache+lucene+main/gradle/java/modules.gradle' line: 215
08:09:37
08:09:37 * What went wrong:
08:09:37 Execution failed for task ':lucene:core.tests:test'.
08:09:37 > java.nio.file.NoSuchFileException: /var/lib/jenkins/workspace/apache+lucene+main/lucene/core.tests/build/tmp/test/jvm-forking.properties
```
I think the problem is that `core.tests` in the middle there, which should instead be a `core/tests`, but I'm not sure if that's something wrong with our Jenkins environment or if its a bug in the gradle logic that is constructing the path.
--
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: issues-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene] uschindler commented on pull request #909: LUCENE-10370: pass proper classpath/module arguments for forking jvms from within tests
Posted by GitBox <gi...@apache.org>.
uschindler commented on PR #909:
URL: https://github.com/apache/lucene/pull/909#issuecomment-1132736446
Could we not fallback to the classpath code if the forkoptions file system property is not there?
This would allow to run tests in most jvms. Maybe print a warning then.
--
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: issues-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene] dweiss commented on pull request #909: LUCENE-10370: pass proper classpath/module arguments for forking jvms from within tests
Posted by GitBox <gi...@apache.org>.
dweiss commented on PR #909:
URL: https://github.com/apache/lucene/pull/909#issuecomment-1132763931
I though about it but... Nah. I don't think it makes sense. You can run tests via gradle from IDEs as well if you have to.
The only thing I was wondering would be whether to throw an assertion error or an assumption error from direct IDE launches.
--
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: issues-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene] dweiss commented on pull request #909: LUCENE-10370: pass proper classpath/module arguments for forking jvms from within tests
Posted by GitBox <gi...@apache.org>.
dweiss commented on PR #909:
URL: https://github.com/apache/lucene/pull/909#issuecomment-1134379491
I've committed a fix.
--
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: issues-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene] dweiss commented on pull request #909: LUCENE-10370: pass proper classpath/module arguments for forking jvms from within tests
Posted by GitBox <gi...@apache.org>.
dweiss commented on PR #909:
URL: https://github.com/apache/lucene/pull/909#issuecomment-1134368823
Hi Alan. Is the Lucene build failing for you locally?
--
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: issues-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene] romseygeek commented on pull request #909: LUCENE-10370: pass proper classpath/module arguments for forking jvms from within tests
Posted by GitBox <gi...@apache.org>.
romseygeek commented on PR #909:
URL: https://github.com/apache/lucene/pull/909#issuecomment-1134383958
Thanks Dawid!
--
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: issues-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene] dweiss commented on pull request #909: LUCENE-10370: pass proper classpath/module arguments for forking jvms from within tests
Posted by GitBox <gi...@apache.org>.
dweiss commented on PR #909:
URL: https://github.com/apache/lucene/pull/909#issuecomment-1133434751
> If we make it an assumption, it makes the results look a bit nicer?
Depends. It can go unnoticed then because it superficially looks just as if the test was ignored. I'd rather have it fail because then it draws more attention.
--
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: issues-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene] dweiss merged pull request #909: LUCENE-10370: pass proper classpath/module arguments for forking jvms from within tests
Posted by GitBox <gi...@apache.org>.
dweiss merged PR #909:
URL: https://github.com/apache/lucene/pull/909
--
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: issues-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene] uschindler commented on pull request #909: LUCENE-10370: pass proper classpath/module arguments for forking jvms from within tests
Posted by GitBox <gi...@apache.org>.
uschindler commented on PR #909:
URL: https://github.com/apache/lucene/pull/909#issuecomment-1134378329
Yes, your Jenkins seems to call `gradlew clean test`, too, so this is failing.
On ASF and Policeman Jenkins we do not do this, so it passes. Jenkins on my managed Jenkins instances have the "reset git reporitoy to clean checkout" feature enabled, so when job starts it has a completely clean git checkout, so `gradlew clean` is not needed.
--
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: issues-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene] dweiss commented on pull request #909: LUCENE-10370: pass proper classpath/module arguments for forking jvms from within tests
Posted by GitBox <gi...@apache.org>.
dweiss commented on PR #909:
URL: https://github.com/apache/lucene/pull/909#issuecomment-1133086554
@uschindler Are you ok with the patch as is? Should I rework it into an assumption error? Should I fall back to classpath if nothing is available (I'm not sold on this one)?
--
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: issues-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene] romseygeek commented on pull request #909: LUCENE-10370: pass proper classpath/module arguments for forking jvms from within tests
Posted by GitBox <gi...@apache.org>.
romseygeek commented on PR #909:
URL: https://github.com/apache/lucene/pull/909#issuecomment-1134364141
Aha, and it's failing locally for me as well. I'll see if I can work out where the issue is!
--
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: issues-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene] dweiss commented on pull request #909: LUCENE-10370: pass proper classpath/module arguments for forking jvms from within tests
Posted by GitBox <gi...@apache.org>.
dweiss commented on PR #909:
URL: https://github.com/apache/lucene/pull/909#issuecomment-1134374008
try Files.createDirectories(forkProperties.toPath().getParent());
--
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: issues-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene] rmuir commented on pull request #909: LUCENE-10370: pass proper classpath/module arguments for forking jvms from within tests
Posted by GitBox <gi...@apache.org>.
rmuir commented on PR #909:
URL: https://github.com/apache/lucene/pull/909#issuecomment-1133125938
i'm suspicious if some of these tests would even run directly from the IDE anyway. even if you fallback to classpath, TestIndexWriterOnJRECrash might not work from IDEA. I installed idea the other day, it defaults to running tests from gradle, so IMO its a non-issue for that IDE. for eclipse it is probably best to simply fail.
--
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: issues-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene] romseygeek commented on pull request #909: LUCENE-10370: pass proper classpath/module arguments for forking jvms from within tests
Posted by GitBox <gi...@apache.org>.
romseygeek commented on PR #909:
URL: https://github.com/apache/lucene/pull/909#issuecomment-1134370810
Hey Dawid, yes I get local failures if I run `./gradlew clean check`.
```
* Where:
Script '/Users/romseygeek/projects/lucene/gradle/java/modules.gradle' line: 215
* What went wrong:
Execution failed for task ':lucene:backward-codecs:test'.
> java.nio.file.NoSuchFileException: /Users/romseygeek/projects/lucene/lucene/backward-codecs/build/tmp/test/jvm-forking.properties
```
I think possibly the problem is that it's not creating the `jvm-forking.properties` file before trying to write to it?
--
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: issues-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene] msokolov commented on pull request #909: LUCENE-10370: pass proper classpath/module arguments for forking jvms from within tests
Posted by GitBox <gi...@apache.org>.
msokolov commented on PR #909:
URL: https://github.com/apache/lucene/pull/909#issuecomment-1133380108
If we make it an assumption, it makes the results look a bit nicer?
--
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: issues-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene] romseygeek commented on pull request #909: LUCENE-10370: pass proper classpath/module arguments for forking jvms from within tests
Posted by GitBox <gi...@apache.org>.
romseygeek commented on PR #909:
URL: https://github.com/apache/lucene/pull/909#issuecomment-1134378224
Can confirm that adding the `createDirectories` line before the `writeString` fixes the problem. Thanks!
--
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: issues-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org