tls: plumb time source to context manager#4810
Conversation
Signed-off-by: Rama <rama.rao@salesforce.com>
|
@lizan can you PTAL? |
jmarantz
left a comment
jmarantz
left a comment
There was a problem hiding this comment.
looks good apart from nits
| TestEnvironment::substitute("{{ test_rundir }}/test/common/ssl/test_data/san_dns_cert.pem")); | ||
| EXPECT_LE(0, Utility::getDaysUntilExpiration(cert.get())); | ||
| Event::SimulatedTimeSystem time_source; | ||
| time_source.setSystemTime(std::chrono::system_clock::from_time_t(1540080000)); |
There was a problem hiding this comment.
comment where that constant comes from and/or declare it as a const.
| ClientContextConfigImpl cfg(tls_context, factory_context_); | ||
| Runtime::MockLoader runtime; | ||
| ContextManagerImpl manager(runtime); | ||
| Event::SimulatedTimeSystem time_system; |
There was a problem hiding this comment.
suggest putting this as a test class member var.
|
@ramaraochavali can you update the description to explain that it's ultimately |
Signed-off-by: Rama <rama.rao@salesforce.com>
Signed-off-by: Rama <rama.rao@salesforce.com>
Signed-off-by: Rama <rama.rao@salesforce.com>
Signed-off-by: Rama <rama.rao@salesforce.com>
|
@jmarantz addressed the comments. PTAL. |
|
|
||
| class SslContextImplTest : public SslCertsTest {}; | ||
| class SslContextImplTest : public SslCertsTest { | ||
| public: |
There was a problem hiding this comment.
fwiw usually 'protected:' is used here as all the test methods are effectively subclasses. Just a suggestion.
| Runtime::MockLoader runtime; | ||
| Event::SimulatedTimeSystem time_system; | ||
| ContextManagerImpl manager(runtime, time_system); | ||
| ContextManagerImpl manager(runtime, time_system_); |
There was a problem hiding this comment.
I guess while in here I would also put runtime and manager in the class to factor out these common initializations but this is pre-existing, so I will leave it up to you.
There was a problem hiding this comment.
I will do a follow-up PR with this
Signed-off-by: Rama <rama.rao@salesforce.com>
4 participants
Description: Plumbs time source to context manager as discussed in #4701. This is to allow control time on
Utility::getDaysUntilExpirationmethod.Risk Level: Low
Testing: Existing automated tests
Docs Changes: N/A
Release Notes: N/A