Re: [2/2] dlls/crypt32: unit test PFXImportCertStore()

[Juan Lang]

Hi Philippe, in addition to the question I had on patch 1/2, I'd like
to know, what is the purpose of this test?

+static void test_createCertificateStore(void)
+{
+    HCERTSTORE certStore = NULL;
+
+    certStore = CertOpenStore(CERT_STORE_PROV_SYSTEM, 0, (HCRYPTPROV)NULL,
+        CERT_SYSTEM_STORE_CURRENT_USER, L"Test Store");
+    ok(certStore != NULL, "could not open the system certificate store\n");
+    if (certStore)
+    {
+        CertCloseStore(certStore, 0);
+    }

This is already covered by tests in store.c, and you're not using
"Test Store" elsewhere in the tests, so I don't see why this is
necessary.

For future reference, L"" string constants aren't portable, don't use
them in the tests.  And, please use CertOpenStoreW if you're going to
use a wide-string parameter to it, or CertOpenStoreA if you're going
to use an ASCII string.

Thanks,
--Juan

[2/2] dlls/crypt32: unit test PFXImportCertStore()

[Philippe Casgrain]

Hi Juan,
> Hi Philippe, in addition to the question I had on patch 1/2, I'd like
> to know, what is the purpose of this test?
>
> +static void test_createCertificateStore(void)
> +{
> - HCERTSTORE certStore = NULL;
> -
> - certStore = CertOpenStore(CERT_STORE_PROV_SYSTEM, 0, (HCRYPTPROV)NULL,
> -
>    CERT_SYSTEM_STORE_CURRENT_USER, L"Test Store");
> - ok(certStore != NULL, "could not open the system certificate store\n");
> - if (certStore)
> - {
> -
>    CertCloseStore(certStore, 0);
> - }
>
> This is already covered by tests in store.c, and you're not using
> "Test Store" elsewhere in the tests, so I don't see why this is
> necessary.

I was doing test-driven development and wanted to start with a store, so I made a test that made sure I could get a store. My environment is set up so that I can only run the "decode" section of the tests (instead of running all the tests all the time, which can be quite slow), hence its inclusion.

I have removed it from this patch for the next submission, and added the wine_todo discussed in the 1/2 message.
> For future reference, L"" string constants aren't portable, don't use
> them in the tests.

Right. That's what I did in the implementation, forgot to do it in the test.
> And, please use CertOpenStoreW if you're going to
> use a wide-string parameter to it, or CertOpenStoreA if you're going
> to use an ASCII string.

Sure. Won't that thunk down to the same thing, though?

Philippe