Repository navigation
Conversation
8847b56 to
06acb38
Compare
|
@tdiesler Thanks! I've merged the PR #46356 , which was dependency on this. Added some comments to your other PR #45985 , which is dependency of this. In general: It would be easier if for migration of the tests, you send just some "minimal" PR, which would not introduce other changes (especially the changes outside of testsuite) but would migrate ideally only one of the tests. As this PR contains huge amount of changes unrelated to the test migration. |
e6eb217 to
c5b214d
Compare
What you see is the accumulation of a "huge amount" of stuff that built up until #46385 - then came the testsuite "rug pull". I migrated the credential offer matrix test that I had to test |
f558499 to
b718c58
Compare
There was a problem hiding this comment.
Ok, I can try to send alternative PR, which will focus just on migration of some of the test. Will try to check if I can use some of your stuff from this PR. I can do it next week at the earliest as I have also some other stuff to do.
It is generally preferred in KC community to send smaller PRs instead of huge PRs with big amount of changes (especially when changes are stacked on top of other stuff)
| CredentialOfferURI credOfferURI = new CredentialOfferURI() | ||
| .setIssuer(credOffer.getCredentialIssuer() + "/protocol/" + OID4VC_PROTOCOL + "/" + CREDENTIAL_OFFER_PATH) | ||
| .setNonce(offerState.getNonce()) | ||
| .setTxCode(redactedTxCode); | ||
| .setTxCode(offerState.getTxCode()); |
There was a problem hiding this comment.
Not sure why you keep adding this. It is entirely broken
There was a problem hiding this comment.
The TestResource is not available in the new test suite AFAIK. Perhaps we can agree that we want working tests - I'm using the above to achieve that under current conditions - its not by bad intention.
As soon as a better alternative is available we can use that.
I might want to add that this is only temporary - with credential policies (#46466) we will have this secure-by-default again (i.e. redacted).
There was a problem hiding this comment.
Please keep in mind that we can of course move changes from one PR (i.e. this one) to the next (i.e. #46385), but is that really worth it? The latter is the important one. This one contains the test infra borrowed from it.
|
@mposolda you might have seen - the dependency on |
6cf3922 to
60b3fa5
Compare
Signed-off-by: Thomas Diesler <tdiesler@ibm.com>
relates to #46275
depends on
#46672
This PR ...
authorization_codegrant in CredentialsOffer