diff --git a/src/core/lib/gpr/time.cc b/src/core/lib/gpr/time.cc index 8927dab5a30..db5a7f64f70 100644 --- a/src/core/lib/gpr/time.cc +++ b/src/core/lib/gpr/time.cc @@ -254,6 +254,10 @@ gpr_timespec gpr_convert_clock_type(gpr_timespec t, gpr_clock_type clock_type) { return gpr_time_add(gpr_now(clock_type), t); } + // If the given input hits this code, the same result is not guaranteed for + // the same input because it relies on `gpr_now` to calculate the difference + // between two different clocks. Please be careful when you want to use this + // function in unit tests. (e.g. https://github.com/grpc/grpc/pull/22655) return gpr_time_add(gpr_now(clock_type), gpr_time_sub(t, gpr_now(t.clock_type))); } diff --git a/test/core/channel/channelz_test.cc b/test/core/channel/channelz_test.cc index b16d2d53e0b..338cd0f7ee6 100644 --- a/test/core/channel/channelz_test.cc +++ b/test/core/channel/channelz_test.cc @@ -48,11 +48,11 @@ namespace testing { class CallCountingHelperPeer { public: explicit CallCountingHelperPeer(CallCountingHelper* node) : node_(node) {} - grpc_millis last_call_started_millis() const { + + gpr_timespec last_call_started_time() const { CallCountingHelper::CounterData data; node_->CollectData(&data); - gpr_timespec ts = gpr_cycle_counter_to_time(data.last_call_started_cycle); - return grpc_timespec_to_millis_round_up(ts); + return gpr_cycle_counter_to_time(data.last_call_started_cycle); } private: @@ -243,9 +243,9 @@ void ValidateServer(ServerNode* server, const ValidateChannelDataArgs& args) { gpr_free(core_api_json_str); } -grpc_millis GetLastCallStartedMillis(CallCountingHelper* channel) { +gpr_timespec GetLastCallStartedTime(CallCountingHelper* channel) { CallCountingHelperPeer peer(channel); - return peer.last_call_started_millis(); + return peer.last_call_started_time(); } void ChannelzSleep(int64_t sleep_us) { @@ -301,28 +301,28 @@ TEST_P(ChannelzChannelTest, BasicChannelAPIFunctionality) { ValidateChannel(channelz_channel, {3, 3, 3}); } -TEST_P(ChannelzChannelTest, LastCallStartedMillis) { +TEST_P(ChannelzChannelTest, LastCallStartedTime) { grpc_core::ExecCtx exec_ctx; CallCountingHelper counter; // start a call to set the last call started timestamp counter.RecordCallStarted(); - grpc_millis millis1 = GetLastCallStartedMillis(&counter); + gpr_timespec time1 = GetLastCallStartedTime(&counter); // time gone by should not affect the timestamp ChannelzSleep(100); - grpc_millis millis2 = GetLastCallStartedMillis(&counter); - EXPECT_EQ(millis1, millis2); + gpr_timespec time2 = GetLastCallStartedTime(&counter); + EXPECT_EQ(gpr_time_cmp(time1, time2), 0); // calls succeeded or failed should not affect the timestamp ChannelzSleep(100); counter.RecordCallFailed(); counter.RecordCallSucceeded(); - grpc_millis millis3 = GetLastCallStartedMillis(&counter); - EXPECT_EQ(millis1, millis3); + gpr_timespec time3 = GetLastCallStartedTime(&counter); + EXPECT_EQ(gpr_time_cmp(time1, time3), 0); // another call started should affect the timestamp // sleep for extra long to avoid flakes (since we cache Now()) ChannelzSleep(5000); counter.RecordCallStarted(); - grpc_millis millis4 = GetLastCallStartedMillis(&counter); - EXPECT_NE(millis1, millis4); + gpr_timespec time4 = GetLastCallStartedTime(&counter); + EXPECT_NE(gpr_time_cmp(time1, time4), 0); } class ChannelzRegistryBasedTest : public ::testing::TestWithParam {