[BEAM-10080] Fix LocalFileSystemsTest to work with Java 11 #12225
Conversation
|
R: @tysonjh |
kennknowles
force-pushed
the
localfilesystem-java11
branch
from
b46be4a
to
bc22394
Compare
|
Failures at https://ci-beam.apache.org/job/beam_PreCommit_Java_Commit/12269/ look like unrelated flakes. |
|
Run Java PreCommit |
|
https://ci-beam.apache.org/job/beam_PreCommit_Java_Phrase/2460/ includes these already-filed flakes:
|
|
Run Java PreCommit |
1 similar comment
|
Run Java PreCommit |
| } | ||
|
|
||
| @VisibleForTesting | ||
| List<MatchResult> match(String pwd, List<String> specs) throws IOException { |
There was a problem hiding this comment.
pwd is a bit unclear as a parameter name.
| // both Windows and correct file resolution. | ||
| // The root cause is that globs are not files but we are using file manipulation libraries | ||
| // to work with them. | ||
| final File relativeFile = new File(spec); |
There was a problem hiding this comment.
Can you change the relativeFile name to something else? It may actually be an absolute file and that would be confusing.
There was a problem hiding this comment.
Renamed to specAsFile since basically it is just cramming the string into a File object, and renamed file into absoluteFile since it is now rooted in a base directory unless it was already absolute.
| } | ||
|
|
||
| @VisibleForTesting | ||
| List<MatchResult> match(String pwd, List<String> specs) throws IOException { |
| // both Windows and correct file resolution. | ||
| // The root cause is that globs are not files but we are using file manipulation libraries | ||
| // to work with them. | ||
| final File relativeFile = new File(spec); |
There was a problem hiding this comment.
Renamed to specAsFile since basically it is just cramming the string into a File object, and renamed file into absoluteFile since it is now rooted in a base directory unless it was already absolute.
kennknowles
force-pushed
the
localfilesystem-java11
branch
from
bc22394
to
40c46be
Compare
|
https://ci-beam.apache.org/job/beam_PreCommit_Java_Commit/12328/ contains two flakes:
|
|
Run Java PreCommit |
|
SparkPortableExecutionTest flake. https://issues.apache.org/jira/browse/BEAM-10460 Opened #12243 to increase the timeout, which is currently 25% higher than some success runs I've seen so there's a chance that it is just a performance or test infra flake. |
|
Run Java PreCommit |
|
Finally green! |
The
LocalFileSystemsTestrelied onSystem.setProperty("user.dir")to set the working directory for file resolution, to simulate a user trying to resolve and stage files relative to where they launch their pipeline.Setting
System.setProperty("user.dir")was never intended to work, but it worked by luck up to and including Java 8. In Java 11 is no longer works. There is no replacement, short of launching a subprocess with a different working directory.We can get adequate test coverage by calling package-private helper methods.
Thank you for your contribution! Follow this checklist to help us incorporate your contribution quickly and easily:
R: @username).[BEAM-XXX] Fixes bug in ApproximateQuantiles, where you replaceBEAM-XXXwith the appropriate JIRA issue, if applicable. This will automatically link the pull request to the issue.CHANGES.mdwith noteworthy changes.See the Contributor Guide for more tips on how to make review process smoother.
Post-Commit Tests Status (on master branch)
Pre-Commit Tests Status (on master branch)
See .test-infra/jenkins/README for trigger phrase, status and link of all Jenkins jobs.