6 tests in Checkstyle master branch fail at clean Ubuntu 14.04 Docker container (Java 8, Maven 3) #3177

Closed
daniilyar opened this Issue May 14, 2016 · 9 comments

Comments

Projects
None yet
3 participants
@daniilyar
Contributor

daniilyar commented May 14, 2016

See more at https://groups.google.com/forum/#!topic/checkstyle-devel/MXmSM98XdfU

Here is the Dockerfile for Checkstyle master branch compilation inside Ubuntu 14.04 container (Java 8, Maven 3.3.9): https://gist.github.com/daniilyar/278fd9023f2a665134308eb07ac1f170

Maven build inside this container fails with (6 test failures):

Failed tests: 
  CheckerTest.testDestroyCacheFileWithInvalidPath:399 Exception did not happen
  MainTest.testExistingFilePlainOutputToFileWithoutReadAndRwPermissions System.exit has not been called.
  MainTest.testExistingTargetFilePlainOutputToFileWithoutRwPermissions System.exit has not been called.
  MainTest.testLoadPropertiesIoException:508 Exception was expected
  PropertyCacheFileTest.testNonAccessibleDirectory:189 AccessDeniedException is expected since directory is readonly.
  PropertyCacheFileTest.testNonAccessibleFile:73 FileNotFoundException is expected, since access to the file was denied!
Tests in error: 
  CommitValidationTest.setUp:106->getCommitsToCheck:142 ? IllegalArgument One of...

Tests run: 1798, Failures: 6, Errors: 1, Skipped: 1

@romani romani added the approved label May 14, 2016

@daniilyar

This comment has been minimized.

Show comment
Hide comment
@daniilyar

daniilyar May 14, 2016

Contributor

I was also able to reproduce this without Docker (locally at clean Ubuntu 14.04 LTS).

Checkstyle master branch (commit 54cdc72c) fails the same tests on my PC (Java 1.8.0_31, Maven 3.2.2). Full build log is here: http://pastebin.com/eEYTNN1F

Contributor

daniilyar commented May 14, 2016

I was also able to reproduce this without Docker (locally at clean Ubuntu 14.04 LTS).

Checkstyle master branch (commit 54cdc72c) fails the same tests on my PC (Java 1.8.0_31, Maven 3.2.2). Full build log is here: http://pastebin.com/eEYTNN1F

@daniilyar daniilyar changed the title from Tests in Checkstyle master branch fail inside clean Ubuntu 14.04 Docker container (Java 8, Maven 3) to 6 tests in Checkstyle master branch fail at clean Ubuntu 14.04 Docker container with Java 8 and Maven 3 May 14, 2016

@daniilyar daniilyar changed the title from 6 tests in Checkstyle master branch fail at clean Ubuntu 14.04 Docker container with Java 8 and Maven 3 to 6 tests in Checkstyle master branch fail at clean Ubuntu 14.04 Docker container (Java 8, Maven 3) May 14, 2016

@MEZk

This comment has been minimized.

Show comment
Hide comment
@MEZk

MEZk May 14, 2016

Contributor

@romani
The same tests failed when I tried to run Checkstyle on Wercker CI with the following DockerFile:
https://github.com/carlossg/docker-maven/blob/40cbcd2edc2719c64062af39baac6ae38d0becf9/jdk-8/Dockerfile

Contributor

MEZk commented May 14, 2016

@romani
The same tests failed when I tried to run Checkstyle on Wercker CI with the following DockerFile:
https://github.com/carlossg/docker-maven/blob/40cbcd2edc2719c64062af39baac6ae38d0becf9/jdk-8/Dockerfile

@romani

This comment has been minimized.

Show comment
Hide comment
@romani

romani May 14, 2016

Member

Good , #3159 depends on this issue. This issue just need to be resolved.

Member

romani commented May 14, 2016

Good , #3159 depends on this issue. This issue just need to be resolved.

@MEZk

This comment has been minimized.

Show comment
Hide comment
@MEZk

MEZk May 26, 2016

Contributor

@romani, @daniilyar
I'm able to correct 2 of 6 UTs:

  1. CheckerTest.testDestroyCacheFileWithInvalidPath
  2. MainTest.testLoadPropertiesIoException

The following UTs remain broken at Docker's container (profile):

Failed tests: 
  MainTest.testExistingFilePlainOutputToFileWithoutReadAndRwPermissions System.exit has not been called.
  MainTest.testExistingTargetFilePlainOutputToFileWithoutRwPermissions System.exit has not been called.
  PropertyCacheFileTest.testNonAccessibleDirectory:189 AccessDeniedException is expected since directory is readonly.
  PropertyCacheFileTest.testNonAccessibleFile:73 FileNotFoundException is expected, since access to the file was denied!

The problem is caused by the fact that file/directory permissions do not set properly.
The following approaches which should set read only permission do not work at all:

final File file = temporaryFolder.newFile("file.output");
file.setReadable(true, false);
file.setWritable(false, false);
final File file = temporaryFolder.newFile("file.output");
file.setReadOnly();
final File file = temporaryFolder.newFile("file.output");
Runtime.getRuntime().exec("chmod 444 " + file.getPath());
final File file = temporaryFolder.newFile("file.output");
Set<PosixFilePermission> perms = new HashSet<>();
perms.add(PosixFilePermission.OWNER_READ);
Files.setPosixFilePermissions(file.toPath(), perms);
final File file = temporaryFolder.newFile("file.output");
chmod(file.getPath(), 444);

Own chmod implementation:

private static int chmod(String filename, int mode) {
    try {
        Class<?> fspClass = Class.forName("java.util.prefs.FileSystemPreferences");
        Method chmodMethod = fspClass.getDeclaredMethod("chmod", String.class, Integer.TYPE);
        chmodMethod.setAccessible(true);
        return (Integer)chmodMethod.invoke(null, filename, mode);
    } catch (Throwable ex) {
        return -1;
    }
}

Nevertheless, they all (1-5) work at my home PC with Ubuntu 14.04 Java 1.8, Maven 3.2.2, at Travis CI, at Appveyor CI.
Any ideas?

FYI:

In *nix system, you may need to configure more specifies about file permission, e.g set a 777 permission for a file or directory, however, Java IO classes do not have ready method for it, but you can use the following dirty workaround :
http://www.mkyong.com/java/how-to-set-the-file-permission-in-java/

http://www.pixelstech.net/article/1440837434-Set-file-permission-in-Java
https://examples.javacodegeeks.com/core-java/io/file/set-file-permissions-in-java-example/
http://stackoverflow.com/questions/664432/how-do-i-programmatically-change-file-permissions

@rnveach
Please, also take a look.

Contributor

MEZk commented May 26, 2016

@romani, @daniilyar
I'm able to correct 2 of 6 UTs:

  1. CheckerTest.testDestroyCacheFileWithInvalidPath
  2. MainTest.testLoadPropertiesIoException

The following UTs remain broken at Docker's container (profile):

Failed tests: 
  MainTest.testExistingFilePlainOutputToFileWithoutReadAndRwPermissions System.exit has not been called.
  MainTest.testExistingTargetFilePlainOutputToFileWithoutRwPermissions System.exit has not been called.
  PropertyCacheFileTest.testNonAccessibleDirectory:189 AccessDeniedException is expected since directory is readonly.
  PropertyCacheFileTest.testNonAccessibleFile:73 FileNotFoundException is expected, since access to the file was denied!

The problem is caused by the fact that file/directory permissions do not set properly.
The following approaches which should set read only permission do not work at all:

final File file = temporaryFolder.newFile("file.output");
file.setReadable(true, false);
file.setWritable(false, false);
final File file = temporaryFolder.newFile("file.output");
file.setReadOnly();
final File file = temporaryFolder.newFile("file.output");
Runtime.getRuntime().exec("chmod 444 " + file.getPath());
final File file = temporaryFolder.newFile("file.output");
Set<PosixFilePermission> perms = new HashSet<>();
perms.add(PosixFilePermission.OWNER_READ);
Files.setPosixFilePermissions(file.toPath(), perms);
final File file = temporaryFolder.newFile("file.output");
chmod(file.getPath(), 444);

Own chmod implementation:

private static int chmod(String filename, int mode) {
    try {
        Class<?> fspClass = Class.forName("java.util.prefs.FileSystemPreferences");
        Method chmodMethod = fspClass.getDeclaredMethod("chmod", String.class, Integer.TYPE);
        chmodMethod.setAccessible(true);
        return (Integer)chmodMethod.invoke(null, filename, mode);
    } catch (Throwable ex) {
        return -1;
    }
}

Nevertheless, they all (1-5) work at my home PC with Ubuntu 14.04 Java 1.8, Maven 3.2.2, at Travis CI, at Appveyor CI.
Any ideas?

FYI:

In *nix system, you may need to configure more specifies about file permission, e.g set a 777 permission for a file or directory, however, Java IO classes do not have ready method for it, but you can use the following dirty workaround :
http://www.mkyong.com/java/how-to-set-the-file-permission-in-java/

http://www.pixelstech.net/article/1440837434-Set-file-permission-in-Java
https://examples.javacodegeeks.com/core-java/io/file/set-file-permissions-in-java-example/
http://stackoverflow.com/questions/664432/how-do-i-programmatically-change-file-permissions

@rnveach
Please, also take a look.

MEZk added a commit to MEZk/checkstyle that referenced this issue May 26, 2016

MEZk added a commit to MEZk/checkstyle that referenced this issue May 26, 2016

MEZk added a commit to MEZk/checkstyle that referenced this issue May 26, 2016

MEZk added a commit to MEZk/checkstyle that referenced this issue May 26, 2016

@MEZk

This comment has been minimized.

Show comment
Hide comment
@MEZk

MEZk May 28, 2016

Contributor

Further investigation.

When I modified 'testNonAccessibleFile' like this:

    @Test
    public void testNonAccessibleFile() throws IOException {
        final Configuration config = new DefaultConfiguration("myName");
        final File file = temporaryFolder.newFile("file.output");
        final boolean readOnly = file.setReadOnly();
        if (readOnly) {
            System.out.println("File is read only!");
        }
        try {
            new PropertyCacheFile(config, file.getAbsolutePath()).persist();
            fail("FileNotFoundException is expected, since access to the file was denied!");
        }
        catch (FileNotFoundException ex) {
            System.out.println("FileNotFoundException in testNonAccessibleFile");
            assertThat(ex.getMessage(), containsString("file.output"));
        }
    }

and ran the test at Wercker CI I had the following output:

-------------------------------------------------------
 T E S T S
-------------------------------------------------------
Running com.puppycrawl.tools.checkstyle.PropertyCacheFileTest
File is read only!
Tests run: 8, Failures: 2, Errors: 0, Skipped: 0, Time elapsed: 1.048 sec <<< FAILURE! - in com.puppycrawl.tools.checkstyle.PropertyCacheFileTest
testNonAccessibleDirectory(com.puppycrawl.tools.checkstyle.PropertyCacheFileTest)  Time elapsed: 0.012 sec  <<< FAILURE!
java.lang.AssertionError: AccessDeniedException is expected since directory is readonly.
    at com.puppycrawl.tools.checkstyle.PropertyCacheFileTest.testNonAccessibleDirectory(PropertyCacheFileTest.

File is read only (as JVM thinks), but information can be written into it in PropertyCacheFile#persist, nevertheless, on my local PC
screenshot at 2016-05-28 14 39 36

How come

Contributor

MEZk commented May 28, 2016

Further investigation.

When I modified 'testNonAccessibleFile' like this:

    @Test
    public void testNonAccessibleFile() throws IOException {
        final Configuration config = new DefaultConfiguration("myName");
        final File file = temporaryFolder.newFile("file.output");
        final boolean readOnly = file.setReadOnly();
        if (readOnly) {
            System.out.println("File is read only!");
        }
        try {
            new PropertyCacheFile(config, file.getAbsolutePath()).persist();
            fail("FileNotFoundException is expected, since access to the file was denied!");
        }
        catch (FileNotFoundException ex) {
            System.out.println("FileNotFoundException in testNonAccessibleFile");
            assertThat(ex.getMessage(), containsString("file.output"));
        }
    }

and ran the test at Wercker CI I had the following output:

-------------------------------------------------------
 T E S T S
-------------------------------------------------------
Running com.puppycrawl.tools.checkstyle.PropertyCacheFileTest
File is read only!
Tests run: 8, Failures: 2, Errors: 0, Skipped: 0, Time elapsed: 1.048 sec <<< FAILURE! - in com.puppycrawl.tools.checkstyle.PropertyCacheFileTest
testNonAccessibleDirectory(com.puppycrawl.tools.checkstyle.PropertyCacheFileTest)  Time elapsed: 0.012 sec  <<< FAILURE!
java.lang.AssertionError: AccessDeniedException is expected since directory is readonly.
    at com.puppycrawl.tools.checkstyle.PropertyCacheFileTest.testNonAccessibleDirectory(PropertyCacheFileTest.

File is read only (as JVM thinks), but information can be written into it in PropertyCacheFile#persist, nevertheless, on my local PC
screenshot at 2016-05-28 14 39 36

How come

MEZk added a commit to MEZk/checkstyle that referenced this issue Jun 2, 2016

Issue #3177: Fix CheckerTest#testDestroyCacheFileWithInvalidPath (Doc…
…ker container: Ubuntu 14.04, Java 8, Maven 3)

MEZk added a commit to MEZk/checkstyle that referenced this issue Jun 2, 2016

MEZk added a commit to MEZk/checkstyle that referenced this issue Jun 2, 2016

romani added a commit that referenced this issue Jun 3, 2016

Issue #3177: Fix CheckerTest#testDestroyCacheFileWithInvalidPath (Doc…
…ker container: Ubuntu 14.04, Java 8, Maven 3)

romani added a commit that referenced this issue Jun 3, 2016

@romani romani added the miscellaneous label Jun 3, 2016

@MEZk

This comment has been minimized.

Show comment
Hide comment
@MEZk

MEZk Jun 3, 2016

Contributor

@romani
After merge of #3221 there are still 4 broken UTs left, all of them try to change file permissions. I'm working on the problem.

Contributor

MEZk commented Jun 3, 2016

@romani
After merge of #3221 there are still 4 broken UTs left, all of them try to change file permissions. I'm working on the problem.

@MEZk

This comment has been minimized.

Show comment
Hide comment
@MEZk

MEZk Jun 16, 2016

Contributor

From canWrite() and canRead() documentation:

On some platforms it may be possible to start the Java virtual machine with special privileges that allow it to modify files that are marked read-only. Consequently this method may return true even though the file is marked read-only.

https://docs.oracle.com/javase/8/docs/api/java/io/File.html#canRead--
https://docs.oracle.com/javase/8/docs/api/java/io/File.html#canWrite--

The following approach also does not work on Linux. A am not able to lock the file.
http://www.studytrails.com/java-io/file-locking.jsp
Explanation
http://stackoverflow.com/questions/11201921/unable-to-lock-files-on-linux-using-java-nio-channels-filelock

Contributor

MEZk commented Jun 16, 2016

From canWrite() and canRead() documentation:

On some platforms it may be possible to start the Java virtual machine with special privileges that allow it to modify files that are marked read-only. Consequently this method may return true even though the file is marked read-only.

https://docs.oracle.com/javase/8/docs/api/java/io/File.html#canRead--
https://docs.oracle.com/javase/8/docs/api/java/io/File.html#canWrite--

The following approach also does not work on Linux. A am not able to lock the file.
http://www.studytrails.com/java-io/file-locking.jsp
Explanation
http://stackoverflow.com/questions/11201921/unable-to-lock-files-on-linux-using-java-nio-channels-filelock

@romani

This comment has been minimized.

Show comment
Hide comment
@romani

romani Jun 17, 2016

Member

@MEZk mentioned that such code appear during #1181 , updated during #1510

We can remove validation from Main.java for RW access, it was done as over-protection that cost nothing for implementation during #1181 , original problem of #1181 was fixed in different way.
It is completely OK to let CLI fail with exception if smth wrong with access to file-system.

Resolution: we can remove validation of Write access from CLI (Main.java) and let validation fail with exception.

Member

romani commented Jun 17, 2016

@MEZk mentioned that such code appear during #1181 , updated during #1510

We can remove validation from Main.java for RW access, it was done as over-protection that cost nothing for implementation during #1181 , original problem of #1181 was fixed in different way.
It is completely OK to let CLI fail with exception if smth wrong with access to file-system.

Resolution: we can remove validation of Write access from CLI (Main.java) and let validation fail with exception.

MEZk added a commit to MEZk/checkstyle that referenced this issue Jun 17, 2016

MEZk added a commit to MEZk/checkstyle that referenced this issue Jun 17, 2016

romani added a commit that referenced this issue Jun 17, 2016

@romani romani added this to the 7.0 milestone Jun 17, 2016

@romani

This comment has been minimized.

Show comment
Hide comment
@romani

romani Jun 17, 2016

Member

fix is merged

Member

romani commented Jun 17, 2016

fix is merged

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment