Community
Participate
Working Groups
Build: I20070501-0010 The fix for bug 105554 adds handling for recursive symlink structures. We should add JUnit tests for these cases to ensure correctness and avoid regressions.
Note that from the test projects attached to bug 105554 comment 40 the test4_udev project is not under EPL and thus cannot be committed to CVS. For a first cut, this test case should be ignored. In the longer term, it might make sense developing a test project that has the same properties (very long link loop, fast growth in breadth during the loop).
We need those tests as precondition for the *working* algorithm for recursive symlinks detection (non working algorithm was developed in bug 105554), which allows users to see they (non recursive) links, see bug 537449.
New Gerrit change created: https://git.eclipse.org/r/126784
I am not able to add the test cases from bug 105554, since the Hudson job will try to archive the test case projects. This fails with an OOM, which comes from no idea where. I've updated the review with some basic test cases, that are simple to understand. The test case which has two recursive symlinks will refresh forever, if the method UnifiedTree.isRecursiveLink() is changed to always return false. So I think the tests still capture the essence of the test cases attached in bug 105554, as far as recursive symlinks are concerned.
(In reply to Simeon Andreev from comment #4) > I am not able to add the test cases from bug 105554, since the Hudson job > will try to archive the test case projects. This fails with an OOM, which > comes from no idea where. Not sure if I understand that. Test case projects are checked into git or created on the fly? In later case, they should be deleted on teardown, so that Hudson is not failing. In former case, they should be changed to be created & deleted on the fly, for the same reason. > I've updated the review with some basic test cases, that are simple to > understand. The test case which has two recursive symlinks will refresh > forever, if the method UnifiedTree.isRecursiveLink() is changed to always > return false. So I think the tests still capture the essence of the test > cases attached in bug 105554, as far as recursive symlinks are concerned. Good.
Some more info on the endless refresh due to recursive symlinks. Without the fix for bug 105554, not any type of recursive symlinks will cause an endless loop in UnifiedTree (at least not under Linux). A simple loop, such as myLink -> ../ will lead to an ever expanding file system path created by the tree. At some point, lstat will be called for a resource with such a path. lstat will return an error code, which will result in no FileInfo for the resource and the endless loop will break. The "endless loop" is instead seen if there are two (or more) recursive symlinks, which both point to the same ancestor directory. The tree which Eclipse will try to build then grows exponentially. The depth is bounded by the path length at which lstat will return an error code, but reaching this depth still takes "forever". (In reply to Andrey Loskutov from comment #5) > (In reply to Simeon Andreev from comment #4) > > I am not able to add the test cases from bug 105554, since the Hudson job > > will try to archive the test case projects. This fails with an OOM, which > > comes from no idea where. > > Not sure if I understand that. Test case projects are checked into git or > created on the fly? In later case, they should be deleted on teardown, so > that Hudson is not failing. In former case, they should be changed to be > created & deleted on the fly, for the same reason. I initially added the test cases attached to bug 105554 in the resources/ folder of the test plug-in (i.e. check in git). I'm not going to go over 100+ files and create the same structure on-the-fly with source code; this is a waste of time in my opinion.
(In reply to Simeon Andreev from comment #6) > I initially added the test cases attached to bug 105554 in the resources/ > folder of the test plug-in (i.e. check in git). I'm not going to go over > 100+ files and create the same structure on-the-fly with source code; this > is a waste of time in my opinion. It depends. If we add "known bad" filesystem examples, we must be aware that not all parts of the tooling could support that. If the test infrastructure doesn't work with recursive links, we have to use a different approach. I think it should be easy to create a tar.gz or zip file out of your "resources" folder and use that for tests. Also please note, that Windows may not allow to create links for a "default" user without admin rights, if I remember it right, there was some limitation earlier, so probably tests need to be done on Mac/Linux only.
(In reply to Andrey Loskutov from comment #7) > I think it should be easy to create a tar.gz or zip file out of your > "resources" folder and use that for tests. Also please note, that Windows > may not allow to create links for a "default" user without admin rights, if > I remember it right, there was some limitation earlier, so probably tests > need to be done on Mac/Linux only. I already do this in our own product tests. However I do the unpacking with unzip --symlinks. I've not been able to unpack the symlinks with JRE API so far.
(In reply to Simeon Andreev from comment #8) > (In reply to Andrey Loskutov from comment #7) > > I think it should be easy to create a tar.gz or zip file out of your > > "resources" folder and use that for tests. Also please note, that Windows > > may not allow to create links for a "default" user without admin rights, if > > I remember it right, there was some limitation earlier, so probably tests > > need to be done on Mac/Linux only. > > I already do this in our own product tests. However I do the unpacking with > unzip --symlinks. I've not been able to unpack the symlinks with JRE API so > far. A-ha. Lett assume we will use unzip and ignore Windows, I hope unzip is available on Mac too?
Gerrit change https://git.eclipse.org/r/126784 was merged to [master]. Commit: http://git.eclipse.org/c/platform/eclipse.platform.resources.git/commit/?id=fa0fc9e32c262a9055f487115b745aee7cbb5660
Simeon, please close tomorrow after SDK tests run (if there will be no surprises).
Mark as resolved.
