Skip to main content

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] [List Home]
Re: [egit-dev] Logical model support in EGit

Hi guys,

Any news about these reviews? I've just updated the existing reviews to rebase them and better reflect their dependencies (I had one backwards, which is why I still had some test failures on https://git.eclipse.org/r/#/c/11966/).

I'd like to get some help with https://git.eclipse.org/r/14968 . Basically, the GitSyncCache is updated a little too eagerly, and I tend to end with a cache that's far from reflecting the true state of my workspace files. The test I provided along with this review shows an obviously wrong behavior :

----------
public void test() {
  [...]
  GitSubscriberResourceMappingContext context = ...;

  assertTrue(context.hasRemoteChange(iFile1, new NullProgressMonitor())); // works as expected (iFile1 has remote change, the method returns true)
  assertTrue(context.hasRemoteChange(iFile2, new NullProgressMonitor())); // works as expected (iFile2 has remote change, the method returns true)

  assertTrue(context.hasRemoteChange(iFile1, new NullProgressMonitor())); // Unexpectedly fails. iFile1 has remote changes, we just checked. Bu the method still returns false
}
----------

This is due to GitSyncObjectCache#merge() resetting the state of any files not being refreshed to "IN-SYNC" (so the second assertion above resets the state of "iFile1" to "IN-SYNC", whatever its real state). Removing this (as does the change on review) will make that test pass... but two others will fail (as can be seen on the report from the automatic hudson job for this change : https://hudson.eclipse.org/sandbox/job/egit.gerrit/4742/).

Could anyone take a look (the original code was from Dariusz Luksza back in 2011 if that helps, https://git.eclipse.org/r/#/c/3891/) and provide me with any lead as to how I could go and fix that properly?

For the five other changes I own, very briefly :
https://git.eclipse.org/r/11928 should now be good.
https://git.eclipse.org/r/13404, https://git.eclipse.org/r/11966 and https://git.eclipse.org/r/13405 all aim at solving the same issue of launching comparisons with the same code from everywhere, considering models along the way.
https://git.eclipse.org/r/14967 and https://git.eclipse.org/r/14966 are probably easier, since they're just the fix for an NPE observed on some configurations and the addition of new tests.

As a side note, I am mostly familiar with the comparison side of things (compare and synchronize), as well as with the corresponding Team's APIs; I'd be willing to become a commiter if that could help in maintaining or helping with the reviews in that part of the code in EGit.

Regards,

Laurent Goubet
Obeo

On 04/06/2013 19:42, Matthias Sohn wrote:
On Tue, Jun 4, 2013 at 1:18 PM, Laurent Goubet <laurent.goubet@xxxxxxx> wrote:
Thanks Matthias,

I've finally managed to get these SWTBot tests working locally, and my latest patch sets "should" solve the test failures.

The builds triggered by these have all failed, but... :

- https://git.eclipse.org/r/#/c/13405/ : only one unrelated failure, a wrong dependency to hamcrest?

 
- https://git.eclipse.org/r/#/c/13404/ : three unrelated failures, two of which seem to be random SWTBot errors.
- https://git.eclipse.org/r/#/c/11966/ : This one should be re-triggered. The failures are related, but wrong : since the two aforementioned changes depend on that one, and they didn't fail, this one should not either (and the previous patch set was actually working)

These changes are no longer drafts and can be reviewed, thanks for the time :).

I'll have a look soon

--
Matthias 

begin:vcard
fn:Laurent Goubet
n:Goubet;Laurent
org:<a href="http://www.obeo.fr";>Obeo</a>
email;internet:laurent.goubet@xxxxxxx
url:http://www.obeo.fr
version:2.1
end:vcard


Back to the top