Stop skipping certain tests under Windows #2130
Comments
|
Thanks for filing the bug!
@cgdecker @cgruber Do we have any way of running our tests regularly on Windows? These shouldn't be hard to fix, but it would be good to have an easy way to test them and a way to make sure errors won't creep back in. |
|
No problem..!:) |
The motivation was my wish for a better ThrowablesTest error message in #2130 (though I'm pretty confident that the problem there is \n vs. \r\n). I made a lot of changes, but I left a lot undone. In particular, I avoided most of the collection assertions, since we often want to test specific collection methods. e.g., we don't necessarily want get() to be rewritten to containsKey(). ------------- Created by MOE: http://code.google.com/p/moe-java MOE_MIGRATED_REVID=101020845
|
Sure, we are really not Windows-enabled here, so we would greatly appreciate any patches. |
|
@kevinb9n, I understand that you cannot run any tests on Windows internally, but I've noticed that for the Github-hosted version of Guava you use Travis CI a lot (I don't know if you also use Travis internally). This makes me wonder if Google policy would allow you to use additional external CI services, and if so whether you'd consider using AppVeyor. It seems to be a rather good complement to Travis, as it allows one to run CI tasks on Windows VMs, and it seems to be free for open source projects. What are your thoughts on this? |
|
On a related note, is the bug in If not, then I have a relatively trivial fix for the bug which I'd be happy to submit a Pull Request for. |
The test in #testGetStackTraceAsString was failing on Windows due to an assumption that lines in a stack trace would always be separated with `\n`, whereas in fact they can be separated with other line separators including `\r\n` (Windows) and `\r` (old Mac). This partially fixes issue google#2130.
|
(Although I've already made the 'fix' on my fork, I understand if it simply does not meet Guava's standards and gets rejected. 😄) |
|
jbduncan got the Error Prone team to successfully adopt Appveyor: google/google-java-format@1961e6a |
|
@cpovirk, did you mean to say "jbduncan got...", or did you mean "jbduncan, (I) got..."? |
|
I think "jbduncan got," as in "You influenced the Error Prone team to do it." In either case, the idea is that we now have an example of how to do this by another team at Google that we know well, so Guava might be able to adopt it more easily, too. |
|
Thanks @cpovirk, it's an honour. :) |
|
OK, we now run CI under Windows. So what's left to do here is to address the remaining failures. The places we need to fix are identifiable from their This should be one of those occasional issues in which we could rubber stamp a PR that addresses one or more of the failures. |
cpovirk
changed the title
Based on [some discussions in GitHub](#6535 (comment)), I'm under the impression that Windows would create the temporary directory/file in a secure location even if our call to `java.nio.file.Files.createTempDirectory`/`createTempFile` passes no ACL attribute. However, in case we are in an unusual situation (Linux with NFS temporary directory???) in which ACLs are supported but the temporary directory is _not_ a secure location, I've arranged for the file to be created with an ACL that grants permissions only to the current user. I set the user's permissions to the ones that I saw on a file created in the temporary directory under Java's default settings, and I didn't do anything to set the additional permissions I was seeing for administrators. The resulting file's permissions look plausibly correct in the Windows property dialog, if slightly different than what I get when creating a file/directory myself through Explorer. Fixes #6535 Relates to: - #4011 - #2575 - #2686 (Yay, we have Windows CI set up!) - #2130 RELNOTES=`io`: Fixed `Files.createTempDir` and `FileBackedOutputStream` under Windows. PiperOrigin-RevId: 538492945
Based on [some discussions in GitHub](#6535 (comment)), I'm under the impression that Windows would create the temporary directory/file in a secure location even if our call to `java.nio.file.Files.createTempDirectory`/`createTempFile` passes no ACL attribute. However, in case we are in an unusual situation (Linux with NFS temporary directory???) in which ACLs are supported but the temporary directory is _not_ a secure location, I've arranged for the file to be created with an ACL that grants permissions only to the current user. I set the user's permissions to the ones that I saw on a file created in the temporary directory under Java's default settings, and I didn't do anything to set the additional permissions I was seeing for administrators. The resulting file's permissions look plausibly correct in the Windows property dialog, if slightly different than what I get when creating a file/directory myself through Explorer. Fixes #6535 Relates to: - #4011 - #2575 - #2686 (Yay, we have Windows CI set up!) - #2130 RELNOTES=`io`: Fixed `Files.createTempDir` and `FileBackedOutputStream` under Windows. PiperOrigin-RevId: 538492945
Based on [some discussions in GitHub](#6535 (comment)), I'm under the impression that Windows would create the temporary directory/file in a secure location even if our call to `java.nio.file.Files.createTempDirectory`/`createTempFile` passes no ACL attribute. However, in case we are in an unusual situation (Linux with NFS temporary directory???) in which ACLs are supported but the temporary directory is _not_ a secure location, I've arranged for the file to be created with an ACL that grants permissions only to the current user. I set the user's permissions to the ones that I saw on a file created in the temporary directory under Java's default settings, and I didn't do anything to set the additional permissions I was seeing for administrators. The resulting file's permissions look plausibly correct in the Windows property dialog, if slightly different than what I get when creating a file/directory myself through Explorer. Fixes #6535 Relates to: - #4011 - #2575 - #2686 (Yay, we have Windows CI set up!) - #2130 RELNOTES=`io`: Fixed `Files.createTempDir` and `FileBackedOutputStream` under Windows. PiperOrigin-RevId: 538492945
Based on [some discussions in GitHub](#6535 (comment)), I'm under the impression that Windows would create the temporary directory/file in a secure location even if our call to `java.nio.file.Files.createTempDirectory`/`createTempFile` passes no ACL attribute. However, in case we are in an unusual situation (Linux with NFS temporary directory???) in which ACLs are supported but the temporary directory is _not_ a secure location, I've arranged for the file to be created with an ACL that grants permissions only to the current user. I set the user's permissions to the ones that I saw on a file created in the temporary directory under Java's default settings, and I didn't do anything to set the additional permissions I was seeing for administrators. The resulting file's permissions look plausibly correct in the Windows property dialog, if slightly different than what I get when creating a file/directory myself through Explorer. Fixes #6535 Relates to: - #4011 - #2575 - #2686 (Yay, we have Windows CI set up!) - #2130 RELNOTES=`io`: Fixed `Files.createTempDir` and `FileBackedOutputStream` under Windows. PiperOrigin-RevId: 538492945
Based on [some discussions in GitHub](#6535 (comment)), I'm under the impression that Windows would create the temporary directory/file in a secure location even if our call to `java.nio.file.Files.createTempDirectory`/`createTempFile` passes no ACL attribute. However, in case we are in an unusual situation (Linux with NFS temporary directory???) in which ACLs are supported but the temporary directory is _not_ a secure location, I've arranged for the file to be created with an ACL that grants permissions only to the current user. I set the user's permissions to the ones that I saw on a file created in the temporary directory under Java's default settings, and I didn't do anything to set the additional permissions I was seeing for administrators. The resulting file's permissions look plausibly correct in the Windows property dialog, if slightly different than what I get when creating a file/directory myself through Explorer. Fixes #6535 Relates to: - #4011 - #2575 - #2686 (Yay, we have Windows CI set up!) - #2130 RELNOTES=`io`: Fixed `Files.createTempDir` and `FileBackedOutputStream` under Windows. PiperOrigin-RevId: 538888769
Some tests depended on Unix line ending, changing those to use `System.lineSeparator()` instead. Now they can run on Windows too.
|
Enabling a few test cases under Windows in #6560. They failed previously due to the use of OS-dependent line endings as you suspected. |
Certain tests were deactivated within AbstractFutureTest on Windows with the explanation that a subset of these test cases run slowly. This is certainly the case, but these tests also exhibit slow performance on Unix. However, they are marginally faster on Unix compared to Windows for some unknown reason. Overall, the difference in execution times doesn't appear to be significant enough to warrant excluding tests on any particular platform.
While on local machine the test cases took roughly similar times both on Windows and on Unix, on GitHub CI the tests under Windows took more than 1 hour (!). Keeping two test cases excluded that were previously measured to be slow.
|
I have managed to narrow down the slow |
I've found them to be [flaky](#6731 (comment)). We already [skip](#2130) some tests under Windows, so what's two more (especially only under Java 8, which I didn't include when setting up [Windows CI](#2686) and which I was testing only as part of #6634)? RELNOTES=n/a PiperOrigin-RevId: 568645480
I've found them to be [flaky](#6731 (comment)). We already [skip](#2130) some tests under Windows, so what's two more (especially only under Java 8, which I didn't include when setting up [Windows CI](#2686) and which I was testing only as part of #6634)? RELNOTES=n/a PiperOrigin-RevId: 570103461
…oogle/guava#2130) and Android, and prepare jimfs for some preliminary Android testing. RELNOTES=n/a PiperOrigin-RevId: 594298059
…2130) and Android, and prepare jimfs for some preliminary Android testing. RELNOTES=n/a PiperOrigin-RevId: 594298059
…oogle/guava#2130) and Android, and prepare jimfs for some preliminary Android testing. RELNOTES=n/a PiperOrigin-RevId: 594346983
…2130) and Android, and prepare jimfs for some preliminary Android testing. RELNOTES=n/a PiperOrigin-RevId: 594346983
Guava tests build is failing on windows. PFB the failure snippet from surefire-reports.
guava-tests/target/surefire-reports/com.google.common.base.ThrowablesTest.txt:Tests run: 30, Failures: 1, Errors: 0, Skipped: 0, Time elapsed: 0.026 sec <<< FAILURE!
Binary file guava-tests/target/surefire-reports/com.google.common.io.ResourcesTest.txt matches
guava-tests/target/surefire-reports/com.google.common.reflect.ClassPathTest.txt:Tests run: 32, Failures: 5, Errors: 0, Skipped: 0, Time elapsed: 0.463 sec <<< FAILURE!
And here is the text for those two files
Test set: com.google.common.base.ThrowablesTest
Tests run: 30, Failures: 1, Errors: 0, Skipped: 0, Time elapsed: 0.026 sec <<< FAILURE!
testGetStackTraceAsString(com.google.common.base.ThrowablesTest) Time elapsed: 0.002 sec <<< FAILURE!
junit.framework.AssertionFailedError: null
at junit.framework.Assert.fail(Assert.java:47)
at junit.framework.Assert.assertTrue(Assert.java:20)
at junit.framework.Assert.assertTrue(Assert.java:27)
at com.google.common.base.ThrowablesTest.testGetStackTraceAsString(ThrowablesTest.java:498)
at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:57)
at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
at java.lang.reflect.Method.invoke(Method.java:606)
at junit.framework.TestCase.runTest(TestCase.java:168)
at junit.framework.TestCase.runBare(TestCase.java:134)
at junit.framework.TestResult$1.protect(TestResult.java:110)
at junit.framework.TestResult.runProtected(TestResult.java:128)
at junit.framework.TestResult.run(TestResult.java:113)
at junit.framework.TestCase.run(TestCase.java:124)
at junit.framework.TestSuite.runTest(TestSuite.java:243)
at junit.framework.TestSuite.run(TestSuite.java:238)
at org.junit.internal.runners.JUnit38ClassRunner.run(JUnit38ClassRunner.java:83)
at org.apache.maven.surefire.junit4.JUnit4TestSet.execute(JUnit4TestSet.java:35)
at org.apache.maven.surefire.junit4.JUnit4Provider.executeTestSet(JUnit4Provider.java:115)
at org.apache.maven.surefire.junit4.JUnit4Provider.invoke(JUnit4Provider.java:97)
at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:57)
at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
at java.lang.reflect.Method.invoke(Method.java:606)
at org.apache.maven.surefire.booter.ProviderFactory$ClassLoaderProxy.invoke(ProviderFactory.java:103)
at com.sun.proxy.$Proxy0.invoke(Unknown Source)
at org.apache.maven.surefire.booter.SurefireStarter.invokeProvider(SurefireStarter.java:150)
at org.apache.maven.surefire.booter.SurefireStarter.runSuitesInProcess(SurefireStarter.java:91)
at org.apache.maven.surefire.booter.ForkedBooter.main(ForkedBooter.java:69)
Test set: com.google.common.reflect.ClassPathTest
Tests run: 32, Failures: 5, Errors: 0, Skipped: 0, Time elapsed: 0.463 sec <<< FAILURE!
testGetClassPathEntry(com.google.common.reflect.ClassPathTest) Time elapsed: 0 sec <<< FAILURE!
junit.framework.AssertionFailedError: expected:file:/C:/usr/test/dep.jar but was:file:/usr/test/dep.jar
at junit.framework.Assert.fail(Assert.java:47)
at junit.framework.Assert.failNotEquals(Assert.java:283)
at junit.framework.Assert.assertEquals(Assert.java:64)
at junit.framework.Assert.assertEquals(Assert.java:71)
at com.google.common.reflect.ClassPathTest.testGetClassPathEntry(ClassPathTest.java:175)
at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:57)
at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
at java.lang.reflect.Method.invoke(Method.java:606)
at junit.framework.TestCase.runTest(TestCase.java:168)
at junit.framework.TestCase.runBare(TestCase.java:134)
at junit.framework.TestResult$1.protect(TestResult.java:110)
at junit.framework.TestResult.runProtected(TestResult.java:128)
at junit.framework.TestResult.run(TestResult.java:113)
at junit.framework.TestCase.run(TestCase.java:124)
at junit.framework.TestSuite.runTest(TestSuite.java:243)
at junit.framework.TestSuite.run(TestSuite.java:238)
at org.junit.internal.runners.JUnit38ClassRunner.run(JUnit38ClassRunner.java:83)
at org.apache.maven.surefire.junit4.JUnit4TestSet.execute(JUnit4TestSet.java:35)
at org.apache.maven.surefire.junit4.JUnit4Provider.executeTestSet(JUnit4Provider.java:115)
at org.apache.maven.surefire.junit4.JUnit4Provider.invoke(JUnit4Provider.java:97)
at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:57)
at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
at java.lang.reflect.Method.invoke(Method.java:606)
at org.apache.maven.surefire.booter.ProviderFactory$ClassLoaderProxy.invoke(ProviderFactory.java:103)
at com.sun.proxy.$Proxy0.invoke(Unknown Source)
at org.apache.maven.surefire.booter.SurefireStarter.invokeProvider(SurefireStarter.java:150)
at org.apache.maven.surefire.booter.SurefireStarter.runSuitesInProcess(SurefireStarter.java:91)
at org.apache.maven.surefire.booter.ForkedBooter.main(ForkedBooter.java:69)
testResourceScanner(com.google.common.reflect.ClassPathTest) Time elapsed: 0.279 sec <<< FAILURE!
-Ankush
The text was updated successfully, but these errors were encountered: