test(*): Adds test to certificate_manager.go #3617
Conversation
Codecov Report
@@ Coverage Diff @@
## main #3617 +/- ##
==========================================
+ Coverage 67.22% 67.27% +0.04%
==========================================
Files 175 177 +2
Lines 8497 8589 +92
==========================================
+ Hits 5712 5778 +66
- Misses 2754 2780 +26
Partials 31 31
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
Adds unit tests to functions in `certificate_manager.go` and resolves openservicemesh#3435. Signed-off-by: Shalier Xia <[email protected]>
shalier
force-pushed
the
testsForCertManager
branch
from
ca158c5 to
4aca35d
Compare
| var errNoPrivateKeyInPEM = errors.New("no private Key in PEM") | ||
|
|
||
| // ErrNoCertificateInPEM is the errror for no certificate in PEM | ||
| var ErrNoCertificateInPEM = errors.New("no certificate in PEM") |
There was a problem hiding this comment.
Is there a reason this is public?
There was a problem hiding this comment.
I think because it is used in another package, certmanager, for testing purpose.
| @@ -52,7 +52,7 @@ func (cm *CertManager) GetCertificate(cn certificate.CommonName) (certificate.Ce | |||
| if cert := cm.getFromCache(cn); cert != nil { | |||
| return cert, nil | |||
| } | |||
| return nil, fmt.Errorf("failed to find certificate with CN=%s", cn) | |||
| return nil, errCertNotFound | |||
There was a problem hiding this comment.
This error is now being used in the vault and cert-manager packages. We could move the errCertNotFound definition to pkg/certificate/providers/errors.go
There was a problem hiding this comment.
Not sure for vault's case, but it creates an import cycle for cert-manager if we do this
Adds unit tests to functions in
certificate_manager.goand resolves #3435.
Signed-off-by: Shalier Xia [email protected]
Description:
Affected area:
Please answer the following questions with yes/no.
Does this change contain code from or inspired by another project? no
Is this a breaking change? no