Adding primary scenario test for Issue #366 #459
Conversation
| @@ -29,6 +29,8 @@ public static class BridgeClientCertificateManager | |||
| private const string ClientCertificateSubject = "WCF Client Certificate"; | |||
| private const string ClientCertificatePassword = "test"; // this needs to be kept in sync with the Bridge configuration | |||
|
|
|||
| public static string Thumbprint { get; private set; } | |||
There was a problem hiding this comment.
Nit: it would be good for a more descriptive name or a comment to describe what this is. Could more than one test attempt to set this to different values?
There was a problem hiding this comment.
+1 to naming.
The cert should be consistent across one test run (i.e., one instance of the Bridge and configuration). If set by multiple tests, should mean that we continue to set it to the same value.
|
LGTM, subject to whether the NIT level questions cause any concern. |
| @@ -176,6 +178,9 @@ public static void InstallLocalCertificateFromBridge() | |||
| } | |||
| } | |||
|
|
|||
| // Set the local variable with the thumbprint so a test case can use it. | |||
| Thumbprint = thumbprint; | |||
There was a problem hiding this comment.
Consider moving this further up where the PUT happens. We already know the cert thumbprint at that point.
|
LGTM |
|
#458 is fixed. If you rebase on top of the changes, you should be fine running these tests now. After rebase, can you please amend the commit to:
|
|
Looks like a rebase is required, but otherwise LGTM. |
* Added a property in BridgeClientCertificateManager to hold the thumbprint of the ClientCertificate so that a test case can get it. * Update initial PR with feedback.
StephenBonikowsky
force-pushed
the
Issue366
branch
from
4e1a24f
to
806cfb4
Compare
|
Getting some errors in ToF, investigating. |
|
I'm getting three API failures from the one test... According to API Catalog all three are supported in NET Core as part of System.ServiceModel.Primitives version 4.1.0.0 I've tried updating wcfopen.settings.targets to explicitly use version 4.1.0.0 but it is getting overridden somehow. @roncain or @iamjasonp @mconnew any ideas where we update the version of S.SM.Primitives for ToF runs? |
|
@hongdai isn't getting these errors, so I will sync and re-build my enlistment. |
|
I can't get K enlistment to build now, because I can confirm that all the types it is complaining about are actually in contract I am going to merge knowing that we have some kind of ToF infrastructure issue related to the version of S.SM.Primitives that is being pulled in. |
Adding primary scenario test for Issue #366
once her PR is merged, before I merge this one.