Re: [PATCH try2 1/3] setupapi/tests: Add rudimentary tests for InstallHinfSectionA/W.

[James Hawkins]

On 6/11/07, Misha Koshelev  wrote:
> Changes from previous version in patch 1:
> - Comments to explain why this conformance test cannot be quite standard - see Notes comment @ beginning
> - Try A function first - I decided that since the A function works on Win98 but not WinXP it is the one
> "broken" (reminder: W works on XP but not 98)
> - Added asserts in helper _InstallHinfSection function to verify A/W function pointers to make sure that
> in the future useW is not misassigned (currently it is assigned properly in START_TEST(install) section as
> it was in previous version).
> - Changed test_install to ok_install to make its purpose more clear. Specifically, the idea is that
> ok_install does the actual install and checks that everything is installed properly, whereas
> test_InstallHinfSection allows us to try using the _InstallHinfSection functions (through ok_install) in
> various ways to test effects of parameters on the installation (e.g., see patch 2)
> - Use property of the "stub" functions of not changing last error to determine which function is stub instead
> of using registry checks
>
> Hopefully this patchset is clearer.

This would still be clearer and more efficient if you set up a
function pointer at the beginning of the tests, and just use that
function for the rest of the tests.  You make a helper function that
tests which version (A or W) works, and assign the working version to
the function pointer.  This way, you don't have to deal with a useW
variable, or any of that mess, and other people can write direct tests
without having to use ok_install (which really shouldn't be needed if
you use a function pointer).  Keep in mind you'll have to use void
pointers for the string parameters when declaring the function pointer
type.

[PATCH try2 1/3] setupapi/tests: Add rudimentary tests for InstallHinfSectionA/W.

[Misha Koshelev]

On Mon, 2007-06-11 at 23:10 -0700, James Hawkins wrote:
> On 6/11/07, Misha Koshelev  wrote:
> > Changes from previous version in patch 1:
> > - Comments to explain why this conformance test cannot be quite standard - see Notes comment @ beginning
> > - Try A function first - I decided that since the A function works on Win98 but not WinXP it is the one
> > "broken" (reminder: W works on XP but not 98)
> > - Added asserts in helper _InstallHinfSection function to verify A/W function pointers to make sure that
> > in the future useW is not misassigned (currently it is assigned properly in START_TEST(install) section as
> > it was in previous version).
> > - Changed test_install to ok_install to make its purpose more clear. Specifically, the idea is that
> > ok_install does the actual install and checks that everything is installed properly, whereas
> > test_InstallHinfSection allows us to try using the _InstallHinfSection functions (through ok_install) in
> > various ways to test effects of parameters on the installation (e.g., see patch 2)
> > - Use property of the "stub" functions of not changing last error to determine which function is stub instead
> > of using registry checks
> >
> > Hopefully this patchset is clearer.
>
> This would still be clearer and more efficient if you set up a
> function pointer at the beginning of the tests, and just use that
> function for the rest of the tests.  You make a helper function that
> tests which version (A or W) works, and assign the working version to
> the function pointer.  This way, you don't have to deal with a useW
> variable, or any of that mess, and other people can write direct tests
> without having to use ok_install (which really shouldn't be needed if
> you use a function pointer).  Keep in mind you'll have to use void
> pointers for the string parameters when declaring the function pointer
> type.

That sounds good but if you are just using one function pointer that
could be set to either the A or W version, don't these other tests still
have to compare the function pointer with each of the separate function
pointers and then do any appropriate unicode or ANSI string conversions
before calling it? Is this really simpler than just calling
_InstallHinfSection which takes all the parameters that
InstallHinfSectionA takes and does appropriate ANSI->Unicode conversion
of the string if it is non-NULL (or passes it directly if it is NULL?).

I suppose if you just mean instead of useW and something like
if (useW) ... else ...
in _InstallHinfSection and other parts I can do
if (funcptr == pInstallHinfSectionW) ... else ...
then I could change this if you really think it will be clearer/easier
to use for other devs. But it seems like you are saving somehow this
level of complexity wouldn't be necessary, and I am not sure I see how.

Thanks
Misha

[PATCH try2 1/3] setupapi/tests: Add rudimentary tests for InstallHinfSectionA/W.

[Misha Koshelev]

On Mon, 2007-06-11 at 23:10 -0700, James Hawkins wrote:
> This would still be clearer and more efficient if you set up a
> function pointer at the beginning of the tests, and just use that
> function for the rest of the tests.  You make a helper function that
> tests which version (A or W) works, and assign the working version to
> the function pointer.  This way, you don't have to deal with a useW
> variable, or any of that mess, and other people can write direct tests
> without having to use ok_install (which really shouldn't be needed if
> you use a function pointer).  Keep in mind you'll have to use void
> pointers for the string parameters when declaring the function pointer
> type.

More succinctly, what would be the advantage of a test function checking
this function pointer you propose and then doing ANSI->Unicode or vice
versa conversions itself over just calling the _InstallHinfSection
function that already exists in my patch and does the appropriate
conversions and then calls the appropriate function?

Thanks
Misha

[PATCH try2 1/3] setupapi/tests: Add rudimentary tests for InstallHinfSectionA/W.

[Misha Koshelev]

On Mon, 2007-06-11 at 23:10 -0700, James Hawkins wrote:
> On 6/11/07, Misha Koshelev  wrote:
> > Changes from previous version in patch 1:
> > - Comments to explain why this conformance test cannot be quite standard - see Notes comment @ beginning
> > - Try A function first - I decided that since the A function works on Win98 but not WinXP it is the one
> > "broken" (reminder: W works on XP but not 98)
> > - Added asserts in helper _InstallHinfSection function to verify A/W function pointers to make sure that
> > in the future useW is not misassigned (currently it is assigned properly in START_TEST(install) section as
> > it was in previous version).
> > - Changed test_install to ok_install to make its purpose more clear. Specifically, the idea is that
> > ok_install does the actual install and checks that everything is installed properly, whereas
> > test_InstallHinfSection allows us to try using the _InstallHinfSection functions (through ok_install) in
> > various ways to test effects of parameters on the installation (e.g., see patch 2)
> > - Use property of the "stub" functions of not changing last error to determine which function is stub instead
> > of using registry checks
> >
> > Hopefully this patchset is clearer.
>
> This would still be clearer and more efficient if you set up a
> function pointer at the beginning of the tests, and just use that
> function for the rest of the tests.  You make a helper function that
> tests which version (A or W) works, and assign the working version to
> the function pointer.  This way, you don't have to deal with a useW
> variable, or any of that mess, and other people can write direct tests
> without having to use ok_install (which really shouldn't be needed if
> you use a function pointer).  Keep in mind you'll have to use void
> pointers for the string parameters when declaring the function pointer
> type.

You know I think you are right in that I can still make it a little
simpler and I think you'll like the result better. I'll post new patches
tonight.

Misha