Skip to content
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.

Sign up for GitHub

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

Jump to bottom

refector & test: update to support new signature.Signer interface #52

Merged
merged 2 commits into from
Aug 26, 2022
Merged

refector & test: update to support new signature.Signer interface #52

merged 2 commits into from
Aug 26, 2022

Conversation

JeyJeyGao
Copy link
Contributor

@JeyJeyGao JeyJeyGao commented Aug 24, 2022 •
edited
Loading

  1. update to support new signature.Signer
  2. update to support new signature.LocalSigner
  3. add remoteSigningMethod & localSigningMethod
  4. update verifyJWT logic to delegate JWT package to select SigningMethod
  5. refector testing structure: extract GetTestLocalSigner() from cose/envelope_test.go to be shared function of testing library in ./signature/signaturetest

Test:

ok github.com/notaryproject/notation-core-go/signature/jws coverage: 96.6% of statements

@JeyJeyGao JeyJeyGao force-pushed the feat/add_jws_test branch 3 times, most recently from 6bb7504 to 692e4bf Compare August 25, 2022 02:36
@JeyJeyGao JeyJeyGao changed the title feat & test: update signature interface refector & test: update signature interface Aug 25, 2022
@JeyJeyGao JeyJeyGao changed the title refector & test: update signature interface refector & test: update signature.Signer interface Aug 25, 2022
@JeyJeyGao JeyJeyGao changed the title refector & test: update signature.Signer interface refector & test: update to support new signature.Signer interface Aug 25, 2022
@JeyJeyGao JeyJeyGao force-pushed the feat/add_jws_test branch 3 times, most recently from 2cd9267 to c97ff24 Compare August 25, 2022 05:39
@chloeyin
Copy link
Contributor

It seems that a lot of failed conditions have been tested, do you test an ok scenario?

chloeyin
chloeyin reviewed Aug 25, 2022
View reviewed changes
signature/jws/jws_test.go Outdated Show resolved Hide resolved
@JeyJeyGao
add jws test
f199296 
Signed-off-by: Junjie Gao <junjiegao@microsoft.com>
chloeyin
chloeyin reviewed Aug 26, 2022
View reviewed changes
signature/jws/envelope.go Outdated Show resolved Hide resolved
@JeyJeyGao JeyJeyGao force-pushed the feat/add_jws_test branch 2 times, most recently from 2f9674f to 02883c9 Compare August 26, 2022 04:40
@JeyJeyGao
update PR
fdf0b9b 
Signed-off-by: Junjie Gao <junjiegao@microsoft.com>
patrickzheng200
patrickzheng200 approved these changes Aug 26, 2022
View reviewed changes
Copy link

@patrickzheng200 patrickzheng200 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@patrickzheng200 patrickzheng200 merged commit 1421844 into notaryproject:cose Aug 26, 2022
JeyJeyGao added a commit to JeyJeyGao/notation-core-go that referenced this pull request Sep 13, 2022
@JeyJeyGao
refector & test: update to support new signature.Signer interface (no…
7071319 
…taryproject#52)

* add jws test

Signed-off-by: Junjie Gao <junjiegao@microsoft.com>

* update PR

Signed-off-by: Junjie Gao <junjiegao@microsoft.com>

Signed-off-by: Junjie Gao <junjiegao@microsoft.com>
Co-authored-by: Junjie Gao <junjiegao@microsoft.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@chloeyin chloeyin chloeyin left review comments

@patrickzheng200 patrickzheng200 patrickzheng200 approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@JeyJeyGao @chloeyin @patrickzheng200