reuse usertokenpolicy used in connect for Reconnect / "ReactivateSession"#3581
reuse usertokenpolicy used in connect for Reconnect / "ReactivateSession"#3581KircMax wants to merge 8 commits intoOPCFoundation:masterfrom
Conversation
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #3581 +/- ##
===========================================
+ Coverage 51.86% 66.10% +14.23%
===========================================
Files 370 459 +89
Lines 78618 97717 +19099
Branches 13650 16377 +2727
===========================================
+ Hits 40779 64597 +23818
+ Misses 33705 28044 -5661
- Partials 4134 5076 +942 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| .GetEndpointAsync(ServerUrl, securityPolicy, Endpoints) | ||
| .ConfigureAwait(false); | ||
| endpoint.Description.SecurityMode = MessageSecurityMode.Sign; | ||
| Assert.NotNull(endpoint); |
There was a problem hiding this comment.
Please add some messages here and to the next Assert.NotNull, this will be hard to debug when something in the Clientfixture changes so e.g. the endpoint is now not available any more.
There was a problem hiding this comment.
I changed to Assert.Ignore which is also done later when the identityPolicy is null so then it'll be ignored in the future (?) or do you prefer a message rather?
romanett
left a comment
romanett
left a comment
There was a problem hiding this comment.
Seems like a reasonable change.
Only issue I see is when the Policy is removed from the Server, but this is a general case that is not handled in reconnect afaik and we have an Issue oipen for that.
|
is there anything more I should do? |
|
@KircMax CI isnt passing yet before we cant merge |
|
@romanett can you please give me a mention once the issue with the failing tests is fixed on master? |
|
@KircMax can you set in Server.AuditEvents.ReportAuditActivateSessionEvent in line 1072 |
|
@romanett I guess that didn't yet do the trick |
|
but since the other 2 PRs now had successful runs of the testcases I guess it might rather actually be related to these changes... |
|
Server did not return an EndpointDescription that matched the one used to create the secure channel. Seems to be the cause, you will investigate the Test cases right? |
…e m_userTokenSecurityPolicyUri, change endpoint configuration only on cloned object
|
@romanett do you think this failure in the last |
3 participants
Proposed changes
Describe the changes here to communicate to the maintainers why we should accept this pull request. If it fixes a bug or resolves a feature request, be sure to link to that issue.
Related Issues
Types of changes
What types of changes does your code introduce?
Put an
xin the boxes that apply. You can also fill these out after creating the PR.Checklist
Put an
xin the boxes that apply. You can also fill these out after creating the PR. If you're unsure about any of them, don't hesitate to ask. We're here to help! This is simply a reminder of what we are going to look for before merging your code.Further comments
If this is a relatively large or complex change, kick off the discussion by explaining why you chose the solution you did and what alternatives you considered, etc...