Re: [4/4] msi/tests: Fix package test when run on a different drive than C:\.

[James Hawkins]

On Tue, Apr 21, 2009 at 2:30 PM, Nicolas Le Cam  wrote:
> This one finally fixes current relative path test to expect correct
> value.

+    drives = GetLogicalDrives();
+    lstrcpyA(path, "A:\");
+    for (i = 0; i < 26; path[0] = '\0', i++)
+    {
+        if (!(drives & (1 << i)))
+            continue;
+
+        path[0] = 'A' + i;
+        if (GetDriveType(path) != DRIVE_FIXED)
+            continue;
+
+        lstrcatA(path + 3, CURR_DIR + 3);
+        attr = GetFileAttributesA(path);
+        if (attr != INVALID_FILE_ATTRIBUTES && (attr &
FILE_ATTRIBUTE_DIRECTORY))
+        {
+            if (path[lstrlenA(path)-1] != '\')
+                lstrcatA(path, "\");
+            break;
+        }
+        path[3] = '\0';
+    }

Same as 2/4.  Please refactor and add many nice comments detailing
exactly what is happening in native msi to make this necessary.

[4/4] msi/tests: Fix package test when run on a different drive than C:\.

[Nicolas Le Cam]

2009/4/21 James Hawkins :
> On Tue, Apr 21, 2009 at 2:30 PM, Nicolas Le Cam  wrote:
> > This one finally fixes current relative path test to expect correct
> > value.
> - drives = GetLogicalDrives();
> - lstrcpyA(path, "A:\");
> - for (i = 0; i < 26; path[0] = '\0', i++)
> - {
> - if (!(drives & (1 << i)))
> - continue;
> -
> - path[0] = 'A' + i;
> - if (GetDriveType(path) != DRIVE_FIXED)
> - continue;
> -
> - lstrcatA(path + 3, CURR_DIR + 3);
> - attr = GetFileAttributesA(path);
> - if (attr != INVALID_FILE_ATTRIBUTES && (attr &
>
> FILE_ATTRIBUTE_DIRECTORY))
> - {
> - if (path[lstrlenA(path)-1] != '\')
> - lstrcatA(path, "\");
> - break;
> - }
> - path[3] = '\0';
> - }
>
> Same as 2/4.  Please refactor and add many nice comments detailing
> exactly what is happening in native msi to make this necessary.
>
> --
> James Hawkins

Hi James, thanks for review.
Will resubmit tomorrow.

To answer your question, basically native msi is doing exactly what
wine is doing (ACTION_SearchDirectory in appsearch.c) except for the
corner case I'm trying to solve, too bad I did find this piece of code
after hours of testing to found out what is native behavior, at least
it has confirmed what I have found :).

[4/4] msi/tests: Fix package test when run on a different drive than C:\.

[James Hawkins]

On Tue, Apr 21, 2009 at 2:51 PM, Nicolas Le Cam  wrote:
> 2009/4/21 James Hawkins :
> > On Tue, Apr 21, 2009 at 2:30 PM, Nicolas Le Cam  wrote:
> > > This one finally fixes current relative path test to expect correct
> > > value.
> > - drives = GetLogicalDrives();
> > - lstrcpyA(path, "A:\");
> > - for (i = 0; i < 26; path[0] = '\0', i++)
> > - {
> > - if (!(drives & (1 << i)))
> > - continue;
> > -
> > - path[0] = 'A' + i;
> > - if (GetDriveType(path) != DRIVE_FIXED)
> > - continue;
> > -
> > - lstrcatA(path + 3, CURR_DIR + 3);
> > - attr = GetFileAttributesA(path);
> > - if (attr != INVALID_FILE_ATTRIBUTES && (attr &
> >
> > FILE_ATTRIBUTE_DIRECTORY))
> > - {
> > - if (path[lstrlenA(path)-1] != '\')
> > - lstrcatA(path, "\");
> > - break;
> > - }
> > - path[3] = '\0';
> > - }
> >
> > Same as 2/4.  Please refactor and add many nice comments detailing
> > exactly what is happening in native msi to make this necessary.
> >
> > --
> > James Hawkins
>
> Hi James, thanks for review.
> Will resubmit tomorrow.
>
> To answer your question, basically native msi is doing exactly what
> wine is doing (ACTION_SearchDirectory in appsearch.c) except for the
> corner case I'm trying to solve, too bad I did find this piece of code
> after hours of testing to found out what is native behavior, at least
> it has confirmed what I have found :).

What I mean is you need a comment detailing the preconditions, the
logic used in msi, and the end result.  Someone with a basic
understanding of the AppSearch action should be able to read the test
and understand what is happening.