Bug 550111 - junit tests are failed on Windows
Summary: junit tests are failed on Windows
Status: NEW
Alias: None
Product: JGit
Classification: Technology
Component: JGit (show other bugs)
Version: 5.5   Edit
Hardware: PC Windows 10
: P3 normal with 1 vote (vote)
Target Milestone: ---   Edit
Assignee: Project Inbox CLA
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2019-08-15 10:59 EDT by Nail Samatov CLA
Modified: 2022-02-07 10:20 EST (History)
5 users (show)

See Also:

Attachments
Close repositories and remove read only files in tests (2.88 KB, application/octet-stream)
2019-08-15 10:59 EDT, Nail Samatov CLA 		no flags Details
View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Nail Samatov CLA 2019-08-15 10:59:53 EDT 
Created attachment 279582 [details]
Close repositories and remove read only files in tests

Overview:
I tried to build jgit on my PC with Windows 10 and found that many junit tests are failed.

Steps to reproduce:
0. Use Windows 10, maven: 3.6.0, java 1.8.
1. Clone jgit repository
2. Go to the folder with jgit sources
3. Run in console: mvn clean install

Actual Results:
Some junit tests are failed.

Expected Results:
All tests should be passed despite that I run them on Windows.

Build info:
I tried it on master branch on commit db0eb9f8aef0beed0a8017d455bf016f2aae7647
and in the branch stable-5.4 on commit 380d7446c9c7ff523f682974b45d83549b66fa9f.
On both junit there are failed junit tests.

My assumtions:
1) Some tests are failed on tearDown() phase because can't delete read only file inside .git folder
2) Another tests are failed on tearDown() phase: can't delete the files because they are locked by current process (because Repository isn't closed)
3) There are other failure reasons (for example different case of expected url), I didn't dig into it yet.

I wonder why it's not reproduced on your machines... Probably Linux/Mac OS behaves differently on these tests and they aren't failed.

I attached the patch that fixes (1) and (2).
I still didn't investigate other cases of test failures on my machine. If my patches are correct and can be applied, I can continue investigation.

Please review the patch and feel free to contact me if additional information is needed.
Comment 1 Matthias Sohn CLA 2019-08-15 11:25:12 EDT 
We know, the main reason for these test failures is that JGit committers all use Mac or Linux. If you are using Windows and want to help to fix these issues you are welcome.

Please follow the contributor guide [1] to push fixes to Gerrit which we use for code reviews.

[1] https://wiki.eclipse.org/EGit/Contributor_Guide#Contributing_Patches
Comment 2 Eclipse Genie CLA 2019-08-21 05:55:40 EDT 
New Gerrit change created: https://git.eclipse.org/r/148039
Comment 3 Eclipse Genie CLA 2021-09-15 14:02:57 EDT 
New Gerrit change created: https://git.eclipse.org/r/c/jgit/jgit/+/185486
Comment 4 Thomas Wolf CLA 2022-01-30 06:08:52 EST 
Some more problems on Windows:

* All tests that run hooks or filters: tests assume a Unix shell

In the core tests:

* BatchRefUpdateTest - LOCK_FAILUREs galore
* OpenSshConfigFileTest - assumes "/tmp/foo" was an absolute path (not true on Windows)
* EolRepositoryTest and DirCacheCheckoutTest.testCheckoutWithLFAuto: EOL differences
* FileReftableTest - unclear; perhaps unclosed streams?
* BinaryDeltaInputStreamTest.testBinaryDelta: java.io.StreamCorruptedException: Binary hunk, line 1: input line not terminated by newline

In the pgm tests:

* LsFilesTest - creates a symlink and fails because of lack of privileges.
* FetchTest and RemoteTest: cannot delete git directory; there are open file handles on a pack file after the repo is closed.
* ProxyConfigTest - not analyzed yet.
Comment 5 Thomas Wolf CLA 2022-01-30 07:23:08 EST 
* ProxyConfigTest -- environment variable names are case-insensitive on Windows.
Comment 6 Thomas Wolf CLA 2022-01-30 07:40:49 EST 
(In reply to Thomas Wolf from comment #4)
> * BinaryDeltaInputStreamTest.testBinaryDelta:
> java.io.StreamCorruptedException: Binary hunk, line 1: input line not
> terminated by newline

Either BinaryHunkInputStream must tolerate lines being terminated by CR-LF, or we need to put a .gitattributes in tst-rsc/org.eclipse.jgit.util.io for the two deltas to be binary. They got checked out with autcrlf=true and thus are CR-LF terminated in the file system.

I think tolerating CR-LF is the more resilient approach.
Comment 7 David Ostrovsky CLA 2022-01-30 11:48:01 EST 
> * FileReftableTest - unclear; perhaps unclosed streams?

//CC Han-Wen Nienhuys

Yes. It is actually a good catch by the tests.

It seems that the current reftable implementation doesn't work on Windows,
because it is deleting files, currently used, in auto compaction code path.

To verify, that it is not just a test issue, I have built Gerrit@HEAD,
deployed it on Windows, created test repository and converted the format
storage to reftable using SSH command:

  $ ssh admin gerrit convert-ref-storage --format reftable -p test_site

as expected the storage format was successfully converted:

  dostr@DESKTOP-MINGW64 /d/pgm/gerrit_site/git/test_site.git
  $ ls reftable/
  000000000001-000000000001-c3475acc.ref  tables.list

If I create change edit in Gerrit for such a repository and try to publish it,
I'm getting exactly the same stack trace in gerrit log as in the failing JUnit tests:

[2022-01-30T17:31:39.233+01:00] [HTTP PUT /changes/test_site~1/edit/Test.java (admin from [0:0:0:0:0:0:0:1])] ERROR com.google.gerrit.httpd.restapi.RestApiServlet : Error in PUT /changes/test_site~1/edit/Test.java: FileSystemException [CONTEXT project="test_site" request="REST /changes/*/edit/*" ]
java.nio.file.FileSystemException: D:\pgm\gerrit_site\git\test_site.git\reftable\000000000001-000000000001-c3475acc.ref: Der Prozess kann nicht auf die Datei zugreifen, da sie von einem anderen Prozess verwendet wird
        at java.base/sun.nio.fs.WindowsException.translateToIOException(WindowsException.java:92)
        at java.base/sun.nio.fs.WindowsException.rethrowAsIOException(WindowsException.java:103)
        at java.base/sun.nio.fs.WindowsException.rethrowAsIOException(WindowsException.java:108)
        at java.base/sun.nio.fs.WindowsFileSystemProvider.implDelete(WindowsFileSystemProvider.java:275)
        at java.base/sun.nio.fs.AbstractFileSystemProvider.delete(AbstractFileSystemProvider.java:105)
        at java.base/java.nio.file.Files.delete(Files.java:1152)
        at org.eclipse.jgit.internal.storage.file.FileReftableStack.compactRange(FileReftableStack.java:531)
        at org.eclipse.jgit.internal.storage.file.FileReftableStack.autoCompact(FileReftableStack.java:686)
        at org.eclipse.jgit.internal.storage.file.FileReftableStack.addReftable(FileReftableStack.java:392)
        at org.eclipse.jgit.internal.storage.file.FileReftableDatabase.addReftable(FileReftableDatabase.java:337)
        at org.eclipse.jgit.internal.storage.file.FileReftableDatabase$FileReftableRefUpdate.doUpdate(FileReftableDatabase.java:479)
        at org.eclipse.jgit.lib.RefUpdate$1.execute(RefUpdate.java:595)
        at org.eclipse.jgit.lib.RefUpdate.updateImpl(RefUpdate.java:735)
        at org.eclipse.jgit.lib.RefUpdate.update(RefUpdate.java:590)
        at org.eclipse.jgit.internal.storage.file.FileReftableDatabase$FileReftableRefUpdate.update(FileReftableDatabase.java:394)
        at com.google.gerrit.server.edit.ChangeEditModifier$NoteDbEdits.updateReference(ChangeEditModifier.java:781)
        at com.google.gerrit.server.edit.ChangeEditModifier$NoteDbEdits.createEdit(ChangeEditModifier.java:730)
        at com.google.gerrit.server.edit.ChangeEditModifier$NewEditBehavior.updateEditInStorage(ChangeEditModifier.java:706)
        at com.google.gerrit.server.edit.ChangeEditModifier.modifyCommit(ChangeEditModifier.java:392)
        at com.google.gerrit.server.edit.ChangeEditModifier.modifyFile(ChangeEditModifier.java:238)
        at com.google.gerrit.server.restapi.change.ChangeEdits$Put.apply(ChangeEdits.java:344)
        at com.google.gerrit.server.restapi.change.ChangeEdits$Create.apply(ChangeEdits.java:134)
        at com.google.gerrit.server.restapi.change.ChangeEdits$Create.apply(ChangeEdits.java:119)

Here is the full stack trace: [1].

Should we file a separate issue for that?

[1] https://paste.opendev.org/show/812439
Comment 8 Thomas Wolf CLA 2022-01-30 11:56:03 EST 
(In reply to David Ostrovsky from comment #7)
> > * FileReftableTest - unclear; perhaps unclosed streams?
> 
> //CC Han-Wen Nienhuys
> 
> Yes. It is actually a good catch by the tests.
> 
> It seems that the current reftable implementation doesn't work on Windows,
> because it is deleting files, currently used, in auto compaction code path.
> [...]
> Should we file a separate issue for that?

Yes, please. On first glance, it appears the current implementation opens InputStreams for the whole stack. How deep is that stack normally? And what happens if you have a Gerrit instance managing thousands of repos with RefTable? Each open input stream consumes a file handle, and typically there are limits how many a process may use.
Comment 9 Eclipse Genie CLA 2022-01-30 11:59:08 EST 
New Gerrit change created: https://git.eclipse.org/r/c/jgit/jgit/+/190181
Comment 10 David Ostrovsky CLA 2022-01-30 12:17:18 EST 
(In reply to Thomas Wolf from comment #8)
> (In reply to David Ostrovsky from comment #7)
> > > * FileReftableTest - unclear; perhaps unclosed streams?
> > 
> > //CC Han-Wen Nienhuys
> > 
> > Yes. It is actually a good catch by the tests.
> > 
> > It seems that the current reftable implementation doesn't work on Windows,
> > because it is deleting files, currently used, in auto compaction code path.
> > [...]
> > Should we file a separate issue for that?
> 
> Yes, please. [...]

Done: [1].

[1] https://bugs.eclipse.org/bugs/show_bug.cgi?id=578454
Comment 12 Eclipse Genie CLA 2022-01-30 15:00:15 EST 
New Gerrit change created: https://git.eclipse.org/r/c/jgit/jgit/+/190182
Comment 13 Eclipse Genie CLA 2022-01-30 15:28:31 EST 
New Gerrit change created: https://git.eclipse.org/r/c/jgit/jgit/+/190183
Comment 14 Eclipse Genie CLA 2022-01-30 15:43:45 EST 
New Gerrit change created: https://git.eclipse.org/r/c/jgit/jgit/+/190184
Comment 15 Eclipse Genie CLA 2022-01-30 16:48:27 EST 
New Gerrit change created: https://git.eclipse.org/r/c/jgit/jgit/+/190186
Comment 21 Thomas Wolf CLA 2022-01-30 18:14:26 EST 
With all these fixes merged I got for JGit core now the following failures:

Bug 578454:

   BatchRefUpdateTest
   FileReftableStackTest
   FileReftableTest

AccessDeniedException when trying to move a loose object's tmp file into place:

   ObjectDirectoryTest

Test error: running an external script:

   AddCommandTest
   CheckoutCommandTest
   RunExternalScriptTest

Test error: EOL (autoCRLF or eol=native):

   EolRepositoryTest

Test error: executable bit:

   RepoCommandTest.testRepoManifestCopyFile:549

Test error: Writing Windows paths with backslashes verbatim into a config file:

  CommitTemplateConfigTest
  ConfigTest

Not analyzed:

   GcConcurrentTest
   FileTreeIteratorTest.testEmptyIfRootIsFile:100
Comment 22 Eclipse Genie CLA 2022-01-31 03:00:20 EST 
New Gerrit change created: https://git.eclipse.org/r/c/jgit/jgit/+/190189
Comment 23 Eclipse Genie CLA 2022-01-31 03:40:03 EST 
New Gerrit change created: https://git.eclipse.org/r/c/jgit/jgit/+/190193
Comment 24 Thomas Wolf CLA 2022-01-31 03:44:11 EST 
(In reply to Eclipse Genie from comment #22)
> New Gerrit change created: https://git.eclipse.org/r/c/jgit/jgit/+/190189

Fixes RepoCommandTest

(In reply to Eclipse Genie from comment #23)
> New Gerrit change created: https://git.eclipse.org/r/c/jgit/jgit/+/190193

Fixes FileTreeIteratorTest.testEmptyIfRootIsFile:100

https://git.eclipse.org/r/c/jgit/jgit/+/190190

Fixes CommitTemplateConfigTest

https://git.eclipse.org/r/c/jgit/jgit/+/190191

Fixes ConfigTest
Comment 29 Eclipse Genie CLA 2022-01-31 18:55:31 EST 
New Gerrit change created: https://git.eclipse.org/r/c/jgit/jgit/+/190215
Comment 30 Thomas Wolf CLA 2022-01-31 18:56:14 EST 
(In reply to Eclipse Genie from comment #29)
> New Gerrit change created: https://git.eclipse.org/r/c/jgit/jgit/+/190215

Fixes EolRepositoryTest.
Comment 32 Nail Samatov CLA 2022-02-07 10:20:04 EST 
Thank you, Thomas!