-
Notifications
You must be signed in to change notification settings - Fork 0
V3 Identity Map test #106
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
V3 Identity Map test #106
Conversation
548b1e6 to
a829112
Compare
|
|
||
| // Need to use the manual mapping for error cases - SDK won't allow creating input with bad emails | ||
| public JsonNode v3IdentityMap(String payload) throws Exception { | ||
| V2Envelope envelope = v2CreateEnvelope(payload, getClientApiSecret()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we can get rid of all the getClientApiKey() or getClientApiSecret() calls and use the consts directly
| return; | ||
| } | ||
|
|
||
| // We need all properties to be there for Identity Map V3, so default all to empty |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We don't need all properties to be present
|
|
||
| // Previous UID is there for 90 days after rotation only, then it's null. | ||
| // If it's there, it should have the correct size | ||
| assertThat(mappedDii.getPreviousRawUid()).satisfiesAnyOf( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We don't want to do an assertion based on the dii's last updated?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We don't expose that to clients so can't get it in e2e test and I don't want to start going to S3 and parsing salt files from e2e tests.
|
|
||
| // Sanity check that refresh from is a date not too far in the past. | ||
| // If it is, either there is an Operator issue or salt rotation hasn't been running for a long time. | ||
| assertThat(mappedDii.getRefreshFrom()).isAfter(Instant.now().minus(Duration.ofDays(10))); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we want to let the test pass if refresh from is in the past at all?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some grace period is useful to ensure the test isn't flaky, but 10 days is probably too much. I'll set it to 1h.
| "suite.operator.TestData#identityMapV3BatchBadEmailArgs", | ||
| "suite.operator.TestData#identityMapV3BatchBadPhoneArgs" | ||
| }) | ||
| public void testV3IdentityMapUnmapped(String label, Operator operator, String operatorName, String payload, String section) throws Exception { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe rename section to identityType?
a829112 to
c0b6854
Compare
No description provided.