Fixing 32-bit test failure

There are some test failures on the 32-bit platforms. It seems the tests were having some issues serialzing and deserialzing SIDs correctly. Adding caching to the tests to avoid serialzing and deserialzing of SIDs.

[pjo, 2014-08-21 20:28:03]

> Adding caching to the tests to avoid serialzing
> and deserialzing of SIDs.

I don't see a removal of serializing or deserializing code.
Are we concerned they were garbage collected?

Thank you,
PJ



- technology's compounding interest
-


On Thu, Aug 21, 2014 at 12:06 PM, <mifern@google.com> wrote:

> Reviewers: myk, pjo, brett.michael.johnson_gmail.com, John L,
>
> Description:
> There are some test failures on the 32-bit platforms. It seems the tests
> were having some issues serialzing and deserialzing SIDs correctly.
> Adding caching to the tests to avoid serialzing and deserialzing of
> SIDs.
>
>
> Please review this at http://codereview.appspot.com/132800043/
>
> Affected files (+6, -1 lines):
>   M test/com/google/enterprise/adaptor/fs/TestWindowsAclViews.java
>
>
> Index: test/com/google/enterprise/adaptor/fs/TestWindowsAclViews.java
> diff --git a/test/com/google/enterprise/adaptor/fs/TestWindowsAclViews.java
> b/test/com/google/enterprise/adaptor/fs/TestWindowsAclViews.java
> index 478593d95db3d47760e250b618f61abfbd585048..
> b6f7aa0285b21c01798e7dd8da43356b4907ee74 100644
> --- a/test/com/google/enterprise/adaptor/fs/TestWindowsAclViews.java
> +++ b/test/com/google/enterprise/adaptor/fs/TestWindowsAclViews.java
> @@ -39,6 +39,7 @@ import java.io.IOException;
>  import java.nio.file.Files;
>  import java.nio.file.Path;
>  import java.util.Arrays;
> +import java.util.HashMap;
>  import java.util.List;
>
>  /**
> @@ -50,6 +51,9 @@ public class TestWindowsAclViews {
>    @Rule
>    public final TemporaryFolder temp = new TemporaryFolder();
>
> +  // Store the SID in a map to avoid serializing and deserialzing them.
> +  static HashMap<Long, AccountSid> nativeToPointer = new HashMap<Long,
> AccountSid>();
> +
>    protected final Path newTempDir(String name) throws IOException {
>      return temp.newFolder(name).toPath().toRealPath();
>    }
> @@ -154,6 +158,7 @@ public class TestWindowsAclViews {
>        sid.write();
>        // See ACCESS_ACEStructure(Pointer p) constructor for mystery
> offsets.
>        memory.setPointer(4 + 4, sid.getPointer());
> +      nativeToPointer.put(Pointer.nativeValue(sid.getPointer()), sid);
>        ace = new Ace(memory);
>        assertEquals(ace.getSID().sid, sid.getPointer());
>        return ace;
> @@ -255,7 +260,7 @@ public class TestWindowsAclViews {
>
>      @Override
>      Account getAccountBySid(WinNT.PSID sid) throws Win32Exception {
> -      return new AccountSid(sid.sid).getAccount();
> +      return nativeToPointer.get(Pointer.nativeValue(sid.sid)).
> getAccount();
>      }
>    }
>  }
>
>
>

[mifern, 2014-08-21 20:39:56]

Nope, I added checks to verify the memory was still allocated (i.e. had not
been freed). The issue seems to be with serialization.

Miguel


On Thu, Aug 21, 2014 at 1:28 PM, PJ <pjo@google.com> wrote:

> > Adding caching to the tests to avoid serialzing
> > and deserialzing of SIDs.
>
> I don't see a removal of serializing or deserializing code.
> Are we concerned they were garbage collected?
>
> Thank you,
> PJ
>
>
>
> - technology's compounding interest
> -
>
>
> On Thu, Aug 21, 2014 at 12:06 PM, <mifern@google.com> wrote:
>
>> Reviewers: myk, pjo, brett.michael.johnson_gmail.com, John L,
>>
>> Description:
>> There are some test failures on the 32-bit platforms. It seems the tests
>> were having some issues serialzing and deserialzing SIDs correctly.
>> Adding caching to the tests to avoid serialzing and deserialzing of
>> SIDs.
>>
>>
>> Please review this at http://codereview.appspot.com/132800043/
>>
>> Affected files (+6, -1 lines):
>>   M test/com/google/enterprise/adaptor/fs/TestWindowsAclViews.java
>>
>>
>> Index: test/com/google/enterprise/adaptor/fs/TestWindowsAclViews.java
>> diff --git a/test/com/google/enterprise/adaptor/fs/TestWindowsAclViews.java
>> b/test/com/google/enterprise/adaptor/fs/TestWindowsAclViews.java
>> index 478593d95db3d47760e250b618f61abfbd585048..
>> b6f7aa0285b21c01798e7dd8da43356b4907ee74 100644
>> --- a/test/com/google/enterprise/adaptor/fs/TestWindowsAclViews.java
>> +++ b/test/com/google/enterprise/adaptor/fs/TestWindowsAclViews.java
>> @@ -39,6 +39,7 @@ import java.io.IOException;
>>  import java.nio.file.Files;
>>  import java.nio.file.Path;
>>  import java.util.Arrays;
>> +import java.util.HashMap;
>>  import java.util.List;
>>
>>  /**
>> @@ -50,6 +51,9 @@ public class TestWindowsAclViews {
>>    @Rule
>>    public final TemporaryFolder temp = new TemporaryFolder();
>>
>> +  // Store the SID in a map to avoid serializing and deserialzing them.
>> +  static HashMap<Long, AccountSid> nativeToPointer = new HashMap<Long,
>> AccountSid>();
>> +
>>    protected final Path newTempDir(String name) throws IOException {
>>      return temp.newFolder(name).toPath().toRealPath();
>>    }
>> @@ -154,6 +158,7 @@ public class TestWindowsAclViews {
>>        sid.write();
>>        // See ACCESS_ACEStructure(Pointer p) constructor for mystery
>> offsets.
>>        memory.setPointer(4 + 4, sid.getPointer());
>> +      nativeToPointer.put(Pointer.nativeValue(sid.getPointer()), sid);
>>        ace = new Ace(memory);
>>        assertEquals(ace.getSID().sid, sid.getPointer());
>>        return ace;
>> @@ -255,7 +260,7 @@ public class TestWindowsAclViews {
>>
>>      @Override
>>      Account getAccountBySid(WinNT.PSID sid) throws Win32Exception {
>> -      return new AccountSid(sid.sid).getAccount();
>> +      return nativeToPointer.get(Pointer.nativeValue(sid.sid)).
>> getAccount();
>>      }
>>    }
>>  }
>>
>>
>>
>

[Brett, 2014-08-21 20:47:19]

LGTM

https://codereview.appspot.com/132800043/diff/1/test/com/google/enterprise/ad...
File test/com/google/enterprise/adaptor/fs/TestWindowsAclViews.java (right):

https://codereview.appspot.com/132800043/diff/1/test/com/google/enterprise/ad...
test/com/google/enterprise/adaptor/fs/TestWindowsAclViews.java:55: static
HashMap<Long, AccountSid> nativeToPointer = new HashMap<Long, AccountSid>();
80 cols.

[Brett, 2014-08-21 20:47:47]

On 2014/08/21 20:47:19, Brett wrote:
> LGTM
> 
>
https://codereview.appspot.com/132800043/diff/1/test/com/google/enterprise/ad...
> File test/com/google/enterprise/adaptor/fs/TestWindowsAclViews.java (right):
> 
>
https://codereview.appspot.com/132800043/diff/1/test/com/google/enterprise/ad...
> test/com/google/enterprise/adaptor/fs/TestWindowsAclViews.java:55: static
> HashMap<Long, AccountSid> nativeToPointer = new HashMap<Long, AccountSid>();
> 80 cols.

Did you test this on 64bit?

[Brett, 2014-08-25 21:17:28]

Committed 25 August 2014 to Filesystem Adaptor:

To https://code.google.com/p/plexi.fs/
   7019203..6b846cd  master -> master

[JohnL, 2014-09-16 01:58:37]

-John L

[pjo, 2014-09-16 03:06:39]

Assuming this msg is a ping.

Brett duplicated this CL and it has been submitted.

Thank you


- technology's compounding interest
-

On Mon, Sep 15, 2014 at 6:58 PM, <jlacey@google.com> wrote:

> -John L
>
> https://codereview.appspot.com/132800043/
>

[Brett, 2014-09-17 17:30:41]

On 2014/09/16 03:06:39, pjo wrote:
> Assuming this msg is a ping.
> 
> Brett duplicated this CL and it has been submitted.
> 
> Thank you
> 
> 
> - technology's compounding interest
> -
> 
> On Mon, Sep 15, 2014 at 6:58 PM, <mailto:jlacey@google.com> wrote:
> 
> > -John L
> >
> > https://codereview.appspot.com/132800043/
> >

If you know how to close or reassign code reviews owned
by Miguel, that would be great.