Fix STF after migration to new resource implementation (ref-point migration 9/10)

[tobous]

Fixes the STF after after the migration of Saros/E to the new resource implementation introduced in #1023. Outdated javadoc and method names that are not directly related to the changed logic will be updated in a separate clean-up PR at the end.

This PR only migrates/fixes the existing test. It does not add new one. I opened #1045 for this.

The tests run locally but currently fail due to issues with the docker STF setup.

Reviewing this PR

I would suggest reviewing this PR commit by commit.

Commits

[REFACTOR][STF] Remove unnecessary modifiers in Constants

[INTERNAL][STF] Adjust references to UI component text after migration

[FIX][STF] Fix logic to use existing directory

Fixes the method to start a session on an existing directory by using
the correct label to look for.

Removes now unnecessary workaround deselecting the previously selected
radio button before selecting a new one. This logic was added to avoid a
bug which is only present in SWTBot versions <2.22.0.

Adds a sleep after switching the radio buttons. This sleep is necessary
to ensure that the internal listener reacting to the change event has
finished updating the UI. Otherwise, entering the text into the
directory path field fails as it is seen as read-only.

Adds constants for the labels to look for in the
AddReferencePointsToSessionWizard.

[FIX][STF] Add methods for test setups requiring Java projects

Adds methods to Util to set up testing sessions explicitly using Java
projects. This is now necessary as the project nature of the inviter is
no longer applied on the invitee side following the migration to the
reference-point-based sharing model.

The methods avoid this issue by first creating a stub module with the
correct nature on the invitee's side and then choose the option of using
an existing project for the negotiation. This imitates the behavior of
creating a new project as part of the resource negotiation while still
correctly setting the project nature.

Adds a note to the documentation of methods that can be used to accept a
resource negotiation using the option to create a new project, stating
that such methods should only be used if the project nature is
irrelevant for the testing setup. Furthermore, the notes point to the
correct methods to use instead if a specific project nature is required.
A similar note is also added to the enum defining the different project
creation types to use.

Removes the methods Util.setUpSessionWithJavaProjectAndClass(...) and
Util.buildFileSessionConcurrently(...) that were no longer being used
and only provided support for no longer supported testing setups.

Adjusts the method Util.setUpSessionWithJavaProjectAndClass(...) to
correctly set up Java projects on the invitee's side. This is also done
by first creating stub projects with the correct nature.

[FIX][STF] Fix tests requiring a Java project nature

Adjusts the STF tests requiring Java projects to use the newly
introduced methods replacing the previous, built-in project nature
support.

Manually creates the project stub on the invitee's side and using the
option to use an existing project in cases where the utility methods
could not be used.

[tobous]

I did not want to rebase the whole stack of PRs just to get the STF to run so I created a merge commit for now. I will remove it from the PR when rebasing later on.

[tobous]

Rebased onto current base branch without any interaction.

[tobous]

Rebased onto current base branch without any interaction.

[tobous]

Rebased onto current base branch without any interaction.

[tobous]

Rebased onto current base branch without any interactions.

[tobous]

Does restrictToReadOnlyAccess wait until Bob is in read-only mode?

I am not sure. But this behavior should not have changed with the move to the reference-point-based model.

Otherwise I find it very questionable to mark every regression issue as flaky.

As I mentioned in the commit message and the comment, I marked this as flaky as I was unable to reproduce this issue locally. I tried running the test through Eclipse on Windows and Linux and through the Docker container (as it is run on the build server), but the test always succeeded for me. So I have no real way of reproducing/debugging this behavior.

Maybe this is due to me only running the specific test. But this would mean that the failure is caused by an interaction with a previous test, e.g. through an incomplete cleanup. But neither the logs nor the screenshots of the failed runs suggested anything of the sorts (at least I couldn't see anything).

Another possibility would be that this is related to the environment the docker container is run in on the build server (e.g. a slower network or fewer computing resources), but this is also very hard to test for.

So I am not really sure what to do here. But, instead of deleting the test, I decided to only mark it as flaky for now as it still serves some value.

To make the situation even worse, rerunning the test now led to a range of test failures cropping up again (see this result/buildscan for the previous run and this results/buildscan for the current run). These failures had already presented themselves before but went away (without any obvious reason) at some point. The worst thing is that now a state that ran successfully before (not counting the test marked as flaky) runs into these issues again, even though absolutely nothing was change on our end (I ran the test on the exact same commit). Of course, I can't reproduce these test failures locally as well. So I really don't know what to do about the STF anymore.

@m273d15 @srossbach Any suggestions?

[srossbach]

What is the issue with this test ? Are the editor contents the same ? If they are equal which content do they display ?

[srossbach]

restrictToReadOnlyAccess actually waits until the remote side has read only access (at least if it is displayed in the GUI). As far as I remember the GUI is updated after the remote side is actually in read-only mode.

So what is the issue ? Does the watchdog not trigger ?

[tobous]

restrictToReadOnlyAccess actually waits until the remote side has read only access (at least if it is displayed in the GUI). As far as I remember the GUI is updated after the remote side is actually in read-only mode.

So what is the issue ? Does the watchdog not trigger ?

The issue with most (if not all) all of the failing tests is that the editor for a file does not open (I think on Bobs side). From the screenshots, I can't really tell whether the file exists or not as the package containing the file is not expanded.

org.eclipse.swtbot.swt.finder.exceptions.WidgetNotFoundException: Timed out waiting for tree item matching [\QFoo1_Saros\E(\Q (shared)\E)?+, \Qsrc\E, \Qmy.pkg\E, \QMyClass.java\E]

As already mentioned, I could not find a way of reproducing this bug and the logs didn't give me much to work on (as I found it very hard to correlate the logs of Alice and Bob to the actual test runs).

[srossbach]

I can't really tell whether the file exists or not as the package containing the file is not expanded.

But it should be expanded. So either a wrong tree object is grabbed or the whole tree object gets replaced and we operate on an outdated one or there is simple a bug in Eclipse regarding the updating or the Package Explorer View.

Again this problem is not new. You could add Flaky to all tests that operate on the package explorer.

So either we find this bug and fix it or we could modify the code in such a way that we simply communicate with the Eclipse API to open specific files in an editor instead of opening them through the GUI.

[srossbach]

BTW are you aware that the cache is no longer working on the current master ?!

INFO  00:48:23,593 [RMI TCP Connection(12)-127.0.0.1] (RemoteBot.java:1662) ******* STARTING TESTCASE saros.stf.test.stf.internal.InternalTest:testcreateFile *******
TRACE 00:48:23,594 [RMI TCP Connection(12)-127.0.0.1] (InternalImpl.java:276) creating java project: Hello
TRACE 00:48:23,603 [RMI TCP Connection(12)-127.0.0.1] (FileSystemChecksumCache.java:93) invalidating checksum for existing file: Z:\java\eclipse\workspace-alice-stf\Hello\.project [0xEB41043A001135E392C8F7232370E4AE]
TRACE 00:48:23,629 [RMI TCP Connection(12)-127.0.0.1] (FileSystemChecksumCache.java:93) invalidating checksum for existing file: Z:\java\eclipse\workspace-alice-stf\Hello\.project [0xEB41043A001135E392C8F7232370E4AE]
TRACE 00:48:23,731 [RMI TCP Connection(12)-127.0.0.1] (FileSystemChecksumCache.java:93) invalidating checksum for existing file: Z:\java\eclipse\workspace-alice-stf\Hello\.classpath [0x1810D5FD334F9DB43B66BBC3C10C40F6]
TRACE 00:48:23,736 [RMI TCP Connection(12)-127.0.0.1] (InternalImpl.java:153) creating file in project 'Hello', path 'test.bar' content: bla
TRACE 00:48:23,748 [RMI TCP Connection(12)-127.0.0.1] (FileSystemChecksumCache.java:93) invalidating checksum for existing file: Z:\java\eclipse\workspace-alice-stf\Hello\test.bar [0x97A319EE56FA0B0730473944B7AD37E3]
TRACE 00:48:23,752 [RMI TCP Connection(12)-127.0.0.1] (InternalImpl.java:153) creating file in project 'Hello', path 'a/b/c/d/e/f/g/abc.def' content: bla
TRACE 00:48:23,752 [RMI TCP Connection(12)-127.0.0.1] (InternalImpl.java:322) [a, b, c, d, e, f, g]
TRACE 00:48:23,768 [RMI TCP Connection(12)-127.0.0.1] (FileSystemChecksumCache.java:93) invalidating checksum for existing file: Z:\java\eclipse\workspace-alice-stf\Hello\a\b\c\d\e\f\g\abc.def [0x1F94868A7B6BDEE0919BFCB30CBE9626]
TRACE 00:48:23,770 [RMI TCP Connection(12)-127.0.0.1] (InternalImpl.java:205) creating file in project 'Hello', path 'x/y/z/foo.bar' size: 100, compressAble=false
TRACE 00:48:23,770 [RMI TCP Connection(12)-127.0.0.1] (InternalImpl.java:322) [x, y, z]
TRACE 00:48:23,779 [RMI TCP Connection(12)-127.0.0.1] (FileSystemChecksumCache.java:93) invalidating checksum for existing file: Z:\java\eclipse\workspace-alice-stf\Hello\x\y\z\foo.bar [0x67E8D3BA6A9B70B7188245C1D2132E79]
TRACE 00:48:23,782 [RMI TCP Connection(12)-127.0.0.1] (InternalImpl.java:205) creating file in project 'Hello', path 'x/y/z/foo.bar.comp' size: 100, compressAble=true
TRACE 00:48:23,782 [RMI TCP Connection(12)-127.0.0.1] (InternalImpl.java:322) [x, y, z]
TRACE 00:48:23,784 [RMI TCP Connection(12)-127.0.0.1] (FileSystemChecksumCache.java:93) invalidating checksum for existing file: Z:\java\eclipse\workspace-alice-stf\Hello\x\y\z\foo.bar.comp [0xB918431B9CE3F5F01AF354645331E888]
TRACE 00:48:23,790 [RMI TCP Connection(12)-127.0.0.1] (InternalImpl.java:237) deleting project: Hello
TRACE 00:48:23,827 [RMI TCP Connection(12)-127.0.0.1] (FileSystemChecksumCache.java:371) failed to obtain absolute path for file : L/Hello/.classpath (EclipseFileImpl)
TRACE 00:48:23,828 [RMI TCP Connection(12)-127.0.0.1] (FileSystemChecksumCache.java:371) failed to obtain absolute path for file : L/Hello/.project (EclipseFileImpl)
TRACE 00:48:23,828 [RMI TCP Connection(12)-127.0.0.1] (FileSystemChecksumCache.java:371) failed to obtain absolute path for file : L/Hello/test.bar (EclipseFileImpl)
TRACE 00:48:23,828 [RMI TCP Connection(12)-127.0.0.1] (FileSystemChecksumCache.java:371) failed to obtain absolute path for file : L/Hello/x/y/z/foo.bar (EclipseFileImpl)
TRACE 00:48:23,828 [RMI TCP Connection(12)-127.0.0.1] (FileSystemChecksumCache.java:371) failed to obtain absolute path for file : L/Hello/x/y/z/foo.bar.comp (EclipseFileImpl)
TRACE 00:48:23,828 [RMI TCP Connection(12)-127.0.0.1] (FileSystemChecksumCache.java:371) failed to obtain absolute path for file : L/Hello/a/b/c/d/e/f/g/abc.def (EclipseFileImpl)
INFO  00:48:23,831 [RMI TCP Connection(12)-127.0.0.1] (RemoteBot.java:1662) ******* TESTCASE saros.stf.test.stf.internal.InternalTest:testcreateFile SUCCEDED *******
INFO  00:48:23,832 [RMI TCP Connection(12)-127.0.0.1] (RemoteBot.java:1662) ******* TESTCASE saros.stf.test.stf.internal.InternalTest:testcreateFile FINISHED *******

Digging through the current code it should not cause any issues as the cache gets correctly invalided if the file is created once again. However if you try to get a checksum for a non existing file there is now a chance to retrieve a dirty value (beside the flooding of the log file with non helpful trace messages).

[tobous]

BTW are you aware that the cache is no longer working on the current master ?!

INFO  00:48:23,593 [RMI TCP Connection(12)-127.0.0.1] (RemoteBot.java:1662) ******* STARTING TESTCASE saros.stf.test.stf.internal.InternalTest:testcreateFile *******
TRACE 00:48:23,594 [RMI TCP Connection(12)-127.0.0.1] (InternalImpl.java:276) creating java project: Hello
TRACE 00:48:23,603 [RMI TCP Connection(12)-127.0.0.1] (FileSystemChecksumCache.java:93) invalidating checksum for existing file: Z:\java\eclipse\workspace-alice-stf\Hello\.project [0xEB41043A001135E392C8F7232370E4AE]
TRACE 00:48:23,629 [RMI TCP Connection(12)-127.0.0.1] (FileSystemChecksumCache.java:93) invalidating checksum for existing file: Z:\java\eclipse\workspace-alice-stf\Hello\.project [0xEB41043A001135E392C8F7232370E4AE]
TRACE 00:48:23,731 [RMI TCP Connection(12)-127.0.0.1] (FileSystemChecksumCache.java:93) invalidating checksum for existing file: Z:\java\eclipse\workspace-alice-stf\Hello\.classpath [0x1810D5FD334F9DB43B66BBC3C10C40F6]
TRACE 00:48:23,736 [RMI TCP Connection(12)-127.0.0.1] (InternalImpl.java:153) creating file in project 'Hello', path 'test.bar' content: bla
TRACE 00:48:23,748 [RMI TCP Connection(12)-127.0.0.1] (FileSystemChecksumCache.java:93) invalidating checksum for existing file: Z:\java\eclipse\workspace-alice-stf\Hello\test.bar [0x97A319EE56FA0B0730473944B7AD37E3]
TRACE 00:48:23,752 [RMI TCP Connection(12)-127.0.0.1] (InternalImpl.java:153) creating file in project 'Hello', path 'a/b/c/d/e/f/g/abc.def' content: bla
TRACE 00:48:23,752 [RMI TCP Connection(12)-127.0.0.1] (InternalImpl.java:322) [a, b, c, d, e, f, g]
TRACE 00:48:23,768 [RMI TCP Connection(12)-127.0.0.1] (FileSystemChecksumCache.java:93) invalidating checksum for existing file: Z:\java\eclipse\workspace-alice-stf\Hello\a\b\c\d\e\f\g\abc.def [0x1F94868A7B6BDEE0919BFCB30CBE9626]
TRACE 00:48:23,770 [RMI TCP Connection(12)-127.0.0.1] (InternalImpl.java:205) creating file in project 'Hello', path 'x/y/z/foo.bar' size: 100, compressAble=false
TRACE 00:48:23,770 [RMI TCP Connection(12)-127.0.0.1] (InternalImpl.java:322) [x, y, z]
TRACE 00:48:23,779 [RMI TCP Connection(12)-127.0.0.1] (FileSystemChecksumCache.java:93) invalidating checksum for existing file: Z:\java\eclipse\workspace-alice-stf\Hello\x\y\z\foo.bar [0x67E8D3BA6A9B70B7188245C1D2132E79]
TRACE 00:48:23,782 [RMI TCP Connection(12)-127.0.0.1] (InternalImpl.java:205) creating file in project 'Hello', path 'x/y/z/foo.bar.comp' size: 100, compressAble=true
TRACE 00:48:23,782 [RMI TCP Connection(12)-127.0.0.1] (InternalImpl.java:322) [x, y, z]
TRACE 00:48:23,784 [RMI TCP Connection(12)-127.0.0.1] (FileSystemChecksumCache.java:93) invalidating checksum for existing file: Z:\java\eclipse\workspace-alice-stf\Hello\x\y\z\foo.bar.comp [0xB918431B9CE3F5F01AF354645331E888]
TRACE 00:48:23,790 [RMI TCP Connection(12)-127.0.0.1] (InternalImpl.java:237) deleting project: Hello
TRACE 00:48:23,827 [RMI TCP Connection(12)-127.0.0.1] (FileSystemChecksumCache.java:371) failed to obtain absolute path for file : L/Hello/.classpath (EclipseFileImpl)
TRACE 00:48:23,828 [RMI TCP Connection(12)-127.0.0.1] (FileSystemChecksumCache.java:371) failed to obtain absolute path for file : L/Hello/.project (EclipseFileImpl)
TRACE 00:48:23,828 [RMI TCP Connection(12)-127.0.0.1] (FileSystemChecksumCache.java:371) failed to obtain absolute path for file : L/Hello/test.bar (EclipseFileImpl)
TRACE 00:48:23,828 [RMI TCP Connection(12)-127.0.0.1] (FileSystemChecksumCache.java:371) failed to obtain absolute path for file : L/Hello/x/y/z/foo.bar (EclipseFileImpl)
TRACE 00:48:23,828 [RMI TCP Connection(12)-127.0.0.1] (FileSystemChecksumCache.java:371) failed to obtain absolute path for file : L/Hello/x/y/z/foo.bar.comp (EclipseFileImpl)
TRACE 00:48:23,828 [RMI TCP Connection(12)-127.0.0.1] (FileSystemChecksumCache.java:371) failed to obtain absolute path for file : L/Hello/a/b/c/d/e/f/g/abc.def (EclipseFileImpl)
INFO  00:48:23,831 [RMI TCP Connection(12)-127.0.0.1] (RemoteBot.java:1662) ******* TESTCASE saros.stf.test.stf.internal.InternalTest:testcreateFile SUCCEDED *******
INFO  00:48:23,832 [RMI TCP Connection(12)-127.0.0.1] (RemoteBot.java:1662) ******* TESTCASE saros.stf.test.stf.internal.InternalTest:testcreateFile FINISHED *******

Digging through the current code it should not cause any issues as the cache gets correctly invalided if the file is created once again. However if you try to get a checksum for a non existing file there is now a chance to retrieve a dirty value (beside the flooding of the log file with non helpful trace messages).

Sorry for the long absence. I was quite busy writing my thesis.

I can't reproduce this issue locally. I am not sure what causes this issue. If the resource were no longer available, IFile.getLocation() would have returned null, so it still has to be available. I am not sure why the new path is given relative to whatever resource L is. Maybe this issue can be avoided by using the full path instead of the absolute path?

Regarding the general STF issue: I will look into whether the approach of manually opening the editor helps. But I am not sure that this is really an upstream issue. The weird behavior is not reproducible on the current master, so it has to at least be influenced by my rework of the Saros/E filesystem implementation.

[srossbach]

As I already said, it does not do any harm but looks very confusing for developers that are new to this project. Just invalidating the full path instead of the absolute path (old behavior) would do the trick.

However there are plenty of your patches that are not yet merged and if I am not mistaken the issue mentioned here does not occur in the current release version.

[tobous]

Rebased onto current base branch without any interaction.

Review comment has been adressed.

[tobous]

Yay, I finally managed to track down the issues with the STF. The problem was caused by some tests being reliant on the project nature of the shared projects being correctly set up on the invitee's side. This seems to be due to some of the STF API differentiating between a file and a class. This differentiation can only be made if the project is correctly registered with a Java nature.

The issue was not reproducible locally as Eclipse does seem to automatically reload the .project file. But apparently this does not always happen. My guess would be that it only happens when the IDE window is refreshed due to a user action (which never happens in the Docker container).

I avoided the issue by adding new methods to specifically set up a shared Java project. In such cases, an empty stub project with a Java nature is first created on the invitee's side. This stub is then used as an existing project during the resource negotiation. This mimics the previous behavior of creating a new project including project nature as part of the resource negotiation.

I also amended the javadoc of the corresponding methods to state that the creation of a new project as part of the resource negotiation is only usable in test setups where the project nature does not matter.

@srossbach @m273d15 @stefaus Could you review the new changes? I would like to merge this before handing in my thesis in early November.

[m273d15]

👍

The base branch was changed.

[tobous]

Rebased onto current master without any interaction.