Skip to content

Commit aba9dc4

Browse files
gaaclarkegaaclarke
authored
Turned back on debug unit tests (flutter#42261)
I refactored the `EXPECT_EXIT` tests since they are unsafe to execute in a process with multiple threads. This leaves `flutter_desktop_darwin_unittests` disabled since it has existing issues. fixes flutter#103757 [C++, Objective-C, Java style guides]: https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style
1 parent 76afb43 commit aba9dc4

File tree

6 files changed

+84
-25
lines changed

6 files changed

+84
-25
lines changed

‎ci/builders/mac_host_engine.json‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,7 @@
5151
"--variant",
5252
"host_debug",
5353
"--type",
54-
"dart",
54+
"dart,engine",
5555
"--engine-capture-core-dump"
5656
],
5757
"script": "flutter/testing/run_tests.py"

‎flow/frame_timings.cc‎

Lines changed: 50 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -103,32 +103,75 @@ size_t FrameTimingsRecorder::GetPictureCacheBytes() const {
103103

104104
void FrameTimingsRecorder::RecordVsync(fml::TimePoint vsync_start,
105105
fml::TimePoint vsync_target) {
106+
fml::Status status = RecordVsyncImpl(vsync_start, vsync_target);
107+
FML_DCHECK(status.ok());
108+
(void)status;
109+
}
110+
111+
void FrameTimingsRecorder::RecordBuildStart(fml::TimePoint build_start) {
112+
fml::Status status = RecordBuildStartImpl(build_start);
113+
FML_DCHECK(status.ok());
114+
(void)status;
115+
}
116+
117+
void FrameTimingsRecorder::RecordBuildEnd(fml::TimePoint build_end) {
118+
fml::Status status = RecordBuildEndImpl(build_end);
119+
FML_DCHECK(status.ok());
120+
(void)status;
121+
}
122+
123+
void FrameTimingsRecorder::RecordRasterStart(fml::TimePoint raster_start) {
124+
fml::Status status = RecordRasterStartImpl(raster_start);
125+
FML_DCHECK(status.ok());
126+
(void)status;
127+
}
128+
129+
fml::Status FrameTimingsRecorder::RecordVsyncImpl(fml::TimePoint vsync_start,
130+
fml::TimePoint vsync_target) {
106131
std::scoped_lock state_lock(state_mutex_);
107-
FML_DCHECK(state_ == State::kUninitialized);
132+
if (state_ != State::kUninitialized) {
133+
return fml::Status(fml::StatusCode::kFailedPrecondition,
134+
"Check failed: state_ == State::kUninitialized.");
135+
}
108136
state_ = State::kVsync;
109137
vsync_start_ = vsync_start;
110138
vsync_target_ = vsync_target;
139+
return fml::Status();
111140
}
112141

113-
void FrameTimingsRecorder::RecordBuildStart(fml::TimePoint build_start) {
142+
fml::Status FrameTimingsRecorder::RecordBuildStartImpl(
143+
fml::TimePoint build_start) {
114144
std::scoped_lock state_lock(state_mutex_);
115-
FML_DCHECK(state_ == State::kVsync);
145+
if (state_ != State::kVsync) {
146+
return fml::Status(fml::StatusCode::kFailedPrecondition,
147+
"Check failed: state_ == State::kVsync.");
148+
}
116149
state_ = State::kBuildStart;
117150
build_start_ = build_start;
151+
return fml::Status();
118152
}
119153

120-
void FrameTimingsRecorder::RecordBuildEnd(fml::TimePoint build_end) {
154+
fml::Status FrameTimingsRecorder::RecordBuildEndImpl(fml::TimePoint build_end) {
121155
std::scoped_lock state_lock(state_mutex_);
122-
FML_DCHECK(state_ == State::kBuildStart);
156+
if (state_ != State::kBuildStart) {
157+
return fml::Status(fml::StatusCode::kFailedPrecondition,
158+
"Check failed: state_ == State::kBuildStart.");
159+
}
123160
state_ = State::kBuildEnd;
124161
build_end_ = build_end;
162+
return fml::Status();
125163
}
126164

127-
void FrameTimingsRecorder::RecordRasterStart(fml::TimePoint raster_start) {
165+
fml::Status FrameTimingsRecorder::RecordRasterStartImpl(
166+
fml::TimePoint raster_start) {
128167
std::scoped_lock state_lock(state_mutex_);
129-
FML_DCHECK(state_ == State::kBuildEnd);
168+
if (state_ != State::kBuildEnd) {
169+
return fml::Status(fml::StatusCode::kFailedPrecondition,
170+
"Check failed: state_ == State::kBuildEnd.");
171+
}
130172
state_ = State::kRasterStart;
131173
raster_start_ = raster_start;
174+
return fml::Status();
132175
}
133176

134177
FrameTiming FrameTimingsRecorder::RecordRasterEnd(const RasterCache* cache) {

‎flow/frame_timings.h‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
#include "flutter/common/settings.h"
1111
#include "flutter/flow/raster_cache.h"
1212
#include "flutter/fml/macros.h"
13+
#include "flutter/fml/status.h"
1314
#include "flutter/fml/time/time_delta.h"
1415
#include "flutter/fml/time/time_point.h"
1516

@@ -114,6 +115,16 @@ class FrameTimingsRecorder {
114115
FrameTiming GetRecordedTime() const;
115116

116117
private:
118+
FML_FRIEND_TEST(FrameTimingsRecorderTest, ThrowWhenRecordBuildBeforeVsync);
119+
FML_FRIEND_TEST(FrameTimingsRecorderTest,
120+
ThrowWhenRecordRasterBeforeBuildEnd);
121+
122+
[[nodiscard]] fml::Status RecordVsyncImpl(fml::TimePoint vsync_start,
123+
fml::TimePoint vsync_target);
124+
[[nodiscard]] fml::Status RecordBuildStartImpl(fml::TimePoint build_start);
125+
[[nodiscard]] fml::Status RecordBuildEndImpl(fml::TimePoint build_end);
126+
[[nodiscard]] fml::Status RecordRasterStartImpl(fml::TimePoint raster_start);
127+
117128
static std::atomic<uint64_t> frame_number_gen_;
118129

119130
mutable std::mutex state_mutex_;

‎flow/frame_timings_recorder_unittests.cc‎

Lines changed: 9 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -2,21 +2,21 @@
22
// Use of this source code is governed by a BSD-style license that can be
33
// found in the LICENSE file.
44

5+
#include <thread>
56
#include "flutter/flow/frame_timings.h"
67
#include "flutter/flow/testing/layer_test.h"
78
#include "flutter/flow/testing/mock_layer.h"
89
#include "flutter/flow/testing/mock_raster_cache.h"
910
#include "third_party/skia/include/core/SkPictureRecorder.h"
1011

11-
#include <thread>
12-
1312
#include "flutter/fml/time/time_delta.h"
1413
#include "flutter/fml/time/time_point.h"
1514

1615
#include "gtest/gtest.h"
1716

1817
namespace flutter {
19-
namespace testing {
18+
19+
using testing::MockRasterCache;
2020

2121
TEST(FrameTimingsRecorderTest, RecordVsync) {
2222
auto recorder = std::make_unique<FrameTimingsRecorder>();
@@ -130,9 +130,9 @@ TEST(FrameTimingsRecorderTest, ThrowWhenRecordBuildBeforeVsync) {
130130
auto recorder = std::make_unique<FrameTimingsRecorder>();
131131

132132
const auto build_start = fml::TimePoint::Now();
133-
EXPECT_EXIT(recorder->RecordBuildStart(build_start),
134-
::testing::KilledBySignal(SIGABRT),
135-
"Check failed: state_ == State::kVsync.");
133+
fml::Status status = recorder->RecordBuildStartImpl(build_start);
134+
EXPECT_FALSE(status.ok());
135+
EXPECT_EQ(status.message(), "Check failed: state_ == State::kVsync.");
136136
}
137137

138138
TEST(FrameTimingsRecorderTest, ThrowWhenRecordRasterBeforeBuildEnd) {
@@ -143,9 +143,9 @@ TEST(FrameTimingsRecorderTest, ThrowWhenRecordRasterBeforeBuildEnd) {
143143
recorder->RecordVsync(st, en);
144144

145145
const auto raster_start = fml::TimePoint::Now();
146-
EXPECT_EXIT(recorder->RecordRasterStart(raster_start),
147-
::testing::KilledBySignal(SIGABRT),
148-
"Check failed: state_ == State::kBuildEnd.");
146+
fml::Status status = recorder->RecordRasterStartImpl(raster_start);
147+
EXPECT_FALSE(status.ok());
148+
EXPECT_EQ(status.message(), "Check failed: state_ == State::kBuildEnd.");
149149
}
150150

151151
#endif
@@ -297,5 +297,4 @@ TEST(FrameTimingsRecorderTest, FrameNumberTraceArgIsValid) {
297297
ASSERT_EQ(actual_arg, expected_arg);
298298
}
299299

300-
} // namespace testing
301300
} // namespace flutter

‎fml/macros.h‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,4 +38,7 @@
3838
TypeName() = delete; \
3939
FML_DISALLOW_COPY_ASSIGN_AND_MOVE(TypeName)
4040

41+
#define FML_FRIEND_TEST(test_case_name, test_name) \
42+
friend class test_case_name##_##test_name##_Test
43+
4144
#endif // FLUTTER_FML_MACROS_H_

‎testing/run_tests.py‎

Lines changed: 10 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -471,13 +471,16 @@ def make_test(name, flags=None, extra_env=None):
471471
# flutter_desktop_darwin_unittests uses global state that isn't handled
472472
# correctly by gtest-parallel.
473473
# https://github.com/flutter/flutter/issues/104789
474-
run_engine_executable(
475-
build_dir,
476-
'flutter_desktop_darwin_unittests',
477-
executable_filter,
478-
shuffle_flags,
479-
coverage=coverage
480-
)
474+
if not os.path.basename(build_dir).startswith('host_debug'):
475+
# Test is disabled for flaking in debug runs:
476+
# https://github.com/flutter/flutter/issues/127441
477+
run_engine_executable(
478+
build_dir,
479+
'flutter_desktop_darwin_unittests',
480+
executable_filter,
481+
shuffle_flags,
482+
coverage=coverage
483+
)
481484
extra_env = {
482485
# pylint: disable=line-too-long
483486
# See https://developer.apple.com/documentation/metal/diagnosing_metal_programming_issues_early?language=objc

0 commit comments

Comments
 (0)