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

chore: add ssl PATCH method unit test #1423

Closed
wants to merge 2 commits into from
Closed

chore: add ssl PATCH method unit test #1423

wants to merge 2 commits into from

Conversation

Jaycean
Copy link
Member

@Jaycean Jaycean commented Feb 4, 2021

Please answer these questions before submitting a pull request

  • add ssl PATCH method unit test

nic-chen
nic-chen reviewed Feb 4, 2021
View reviewed changes
api/internal/handler/ssl/ssl_test.go
ssl := &entity.SSL{}
reqBody := `{
"id":"1",
"key": "-----BEGIN PRIVATE KEY-----\nMIIEvwIBADANBgkqhkiG9w0BAQEFAASCBKkwggSlAgEAAoIBAQDGO0J9xrOcmvgh\npkqHIYHCw35FTfIT5uXOSzdF49M2ZAKBQwFG0ovYT8bc0glNLB+hpDhJPL531qSP\nl1ZOe0W1ofP1u0T5Zzc9Rub/kn7RMPq0BsSC6J3rF+rQEwh1PM8qUuD8DxZ7jaOL\niMNL6SyuZIPsS1kPPBtsioukdo666tbjNMixhQbI9Wpg55abdXRFh3i7Zu/9siF1\njCGcsskjOaUOY4sYQ3i5WU/HIIRhA82XuIL+Sxd32P8bKi2UT1sqFXRjAVR7KRWo\nIVvkmSLoZb9ucV6MsccDrRYBf6rLbI1tFj9l2rY6GTFlT+6z7K/ZI60DGi/hsBfl\nDeEQ5WuxAgMBAAECggEAVHQQyucpxHGdfzCKlfGnh+Oj20Du/p2jkHUpEkSSypxn\nGM0EMTkoTTsHvTJath8zRrlhJYqUlxfCOk6+fWc1dsGN30Yuh5b6yMd5SK8QCm20\nkZhEhoU2Kl+hMY66TsBefmia46hF6tOYNq1IjwHDgHTgY35ibgQsptyLy8Ca5HTC\nrnoocP2AcKtM+qwOMGiNHpeh+/zfB91C9AszvS8H2ao5nq4u0/JavPO4A4WmVYol\n7Qv9ACY/8uaKC79syahutbkMjwGsQgYsq9G0QpcLSCuOb4vBbOb130mptSM9NzKg\nTjSxF2D8ob//roZMc1ueTpqAY6WedKV3y3BIBDKuAQKBgQDgGyEsxwR9QtA5EH/h\nJ4GiTQn0aep8G2LSlAtHGndL3sxaGGLt2pk3lNIeRAbOS3APmYskBN418JIF/Ren\nE0CYSrTaxpTs9UXXkgKNJ63Z6r+btswTAVVXG5Zoi/5JRSHRquEVmKccM4zg3v6R\ny/nVhwXigUaRuLx+wCtoaGsaUQKBgQDicXFZ0TvN8tohqc8dbmOu2A25+ifFKHUA\nn3yxZIJtbTC9bJeuwtkqIFol1DXHLqYvdD5jQT3c4z6HekcmI9sEy1YzO4a3WUTI\nP//ogjDLXj402k+WCx1Us2HASxwU5cRvOpMhfnppYPSDXqBoH196UCDmOQuS1+Q8\njyPsNQmDYQKBgQDcm5hCvf87V4QmSIm6GOvR20iLY6BCX6seZEHd0r3Q4BgGMK9i\nOahOQJ++z3Rrq3M6yAligbBFJPZ6ErUv8RHLWO9D1exQfvorxT3huke3lxDbtkya\nANwDjdK4Q+ckNXufLDm6yrTmXBC4ZIvw9fyQKASw/lV7qYFUvNN+Shv0oQKBgQC+\nraw3Z7smV0NbaXRgYh5KkuAsJPvsR38OwT3s2qgBoRqTx6eKn8Tidk+y3xlR2nRS\nLV6DkeKX6Ds1NcBH25WIWfkCNzPfnKoQveOuVELmXTugody2ijFuq4a6uASzjC93\nQim24JwPtHbxUHNeelyZ0HODqbGXO3iTji0/sAGMwQKBgQC8yDwapXgrCWK34qpN\nSdO9uA4VstI3Ovb+o3Evfp1CvJnfk56ypO2DaqbuvMJsInuWRFU40UWp7Vxyl/hP\nXvGgEI3dbBy9KWFjAKfI2Wv3i+zvJ1mAHM3u1jcX3zxOxSAN4LJVBudgkGpop1ps\nW5tWveXiXwxCUE/r9ax4mfJvXQ==\n-----END PRIVATE KEY-----",
Copy link
Member

Choose a reason for hiding this comment

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

these test data could put into a file in dir testdata , this make the code look clearer

@codecov-io
Copy link

codecov-io commented Feb 4, 2021 •
edited
Loading

Codecov Report

Merging #1423 (b04e5ad) into master (0a0a42a) will increase coverage by 0.27%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1423      +/-   ##
==========================================
+ Coverage   66.59%   66.87%   +0.27%     
==========================================
  Files          44       44              
  Lines        2937     2937              
==========================================
+ Hits         1956     1964       +8     
+ Misses        744      733      -11     
- Partials      237      240       +3
Impacted Files Coverage Δ
api/internal/utils/closer.go 33.33% <0.00%> (ø)
api/internal/core/storage/etcd.go 48.48% <0.00%> (+4.04%) ⬆️
api/cmd/managerapi.go 60.29% <0.00%> (+5.88%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 0a0a42a...b04e5ad. Read the comment docs.

@juzhiyuan
Copy link
Member

waiting for #1426

@juzhiyuan
Copy link
Member

may sync the latest codes to fix E2E failed.

@Jaycean
Copy link
Member Author

Jaycean commented Feb 4, 2021

may sync the latest codes to fix E2E failed.

no problem.

@Jaycean Jaycean force-pushed the add_ssl_unit_test_of_patch_mothod branch from b04e5ad to fcc3244 Compare February 5, 2021 00:59
@Jaycean
Copy link
Member Author

Jaycean commented Feb 5, 2021

ping @starsz
Hi, I was adding the unit test of patch method before, but I see that you are refactoring the unit test. Have you started the unit test of SSL? If it has started, my PR is also closed. If it has not started, I can help with the refactoring of SSL unit test.

@starsz
Copy link
Contributor

starsz commented Feb 5, 2021

ping @starsz
Hi, I was adding the unit test of patch method before, but I see that you are refactoring the unit test. Have you started the unit test of SSL? If it has started, my PR is also closed. If it has not started, I can help with the refactoring of SSL unit test.

Pretty good. I haven't started refactoring SSL unit test. I need your help 😄.
Thank you very much.

@Jaycean
Copy link
Member Author

Jaycean commented Feb 5, 2021 •
edited
Loading

ping @starsz
Hi, I was adding the unit test of patch method before, but I see that you are refactoring the unit test. Have you started the unit test of SSL? If it has started, my PR is also closed. If it has not started, I can help with the refactoring of SSL unit test.

Pretty good. I haven't started refactoring SSL unit test. I need your help .
Thank you very much.

OK, I'll discuss with you to avoid our repeated work.

@Jaycean
Copy link
Member Author

Jaycean commented Feb 5, 2021

This PR is closed first, and then re open a new pr.

@Jaycean Jaycean closed this Feb 5, 2021
@Jaycean Jaycean deleted the add_ssl_unit_test_of_patch_mothod branch February 5, 2021 09:13
@imjoey imjoey mentioned this pull request Feb 6, 2021
Merged
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nic-chen nic-chen nic-chen left review comments

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

Successfully merging this pull request may close these issues.
5 participants
@Jaycean @codecov-io @juzhiyuan @starsz @nic-chen