Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.

Remove noisy log from startup #35954

Merged
auto-submit merged 1 commit into from
Sep 7, 2022
Merged

Remove noisy log from startup #35954

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 0 additions & 6 deletions shell/platform/embedder/embedder.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -132,12 +132,6 @@ static bool IsOpenGLRendererConfigValid(const FlutterRendererConfig* config) {
return false;
}

if (!SAFE_EXISTS(open_gl_config, populate_existing_damage)) {
FML_LOG(INFO) << "populate_existing_damage was not defined, disabling "
"partial repaint. If you wish to enable partial repaint, "
"please define this callback.";
}
Copy link
Copy Markdown
Member Author

@loic-sharma loic-sharma Sep 6, 2022

Choose a reason for hiding this comment

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

The embedder API has lots of other optional callbacks that no-op if they aren't provided. The embedder API does not log a message if those other optional callbacks aren't provided.


return true;
}

Expand Down
26 changes: 26 additions & 0 deletions shell/platform/windows/flutter_windows_unittests.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,32 @@ TEST_F(WindowsTest, LaunchMain) {
ASSERT_NE(controller, nullptr);
}

// Verify there is no unexpected output from launching main.
TEST_F(WindowsTest, LaunchMainHasNoOutput) {
// Replace stdout & stderr stream buffers with our own.
std::stringstream cout_buffer;
std::stringstream cerr_buffer;
std::streambuf* old_cout_buffer = std::cout.rdbuf();
std::streambuf* old_cerr_buffer = std::cerr.rdbuf();
std::cout.rdbuf(cout_buffer.rdbuf());
std::cerr.rdbuf(cerr_buffer.rdbuf());

auto& context = GetContext();
WindowsConfigBuilder builder(context);
ViewControllerPtr controller{builder.Run()};
ASSERT_NE(controller, nullptr);

// Restore original stdout & stderr stream buffer.
std::cout.rdbuf(old_cout_buffer);
std::cerr.rdbuf(old_cerr_buffer);

// Verify stdout & stderr have no output.
std::string cout = cout_buffer.str();
std::string cerr = cerr_buffer.str();
EXPECT_TRUE(cout.empty());
EXPECT_TRUE(cerr.empty());
Copy link
Copy Markdown
Member

@cbracken cbracken Sep 7, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

You can use testing::internal::CaptureStdout() and testing::internal::GetCapturedStdout() to make this more succinct and avoid the possibility of copy/paste and forgetting to restore at the end.

Copy link
Copy Markdown
Member Author

@loic-sharma loic-sharma Sep 7, 2022

Choose a reason for hiding this comment

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

I considered this but then found this message by the gtest folks: https://groups.google.com/g/googletestframework/c/7OTybZo-jp8

First of all, you should not use APIs marked as "internal".
They are not part of the public API, are not supported and are subject to change at any time.

And:

No, those APIs are internal for testing gTest itself.
They do so by overriding the stdout/stderr file descriptors and forwarding the data into a file, which means that they interfere with the actual output of the binary.
They could forward the output to the actual stdout/err, but that is more complex and there is no reason to do so on these internal APIs.

It is rather trivial to capture std::cout/std::cerr (not stdout/stderr) on your side, if you want.
Eg: http://stackoverflow.com/questions/5419356/redirect-stdout-stderr-to-a-string

I think if this becomes a common pattern we could introduce a helper to prevent possible gotchas.

Copy link
Copy Markdown
Member Author

@loic-sharma loic-sharma Sep 7, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

I accidentally added the autosubmit to this before letting you reply. Let me know if you have feedback, I'd be happy to send a follow-up pull request. Sorry about that!

Copy link
Copy Markdown
Member

@cbracken cbracken Sep 7, 2022

Choose a reason for hiding this comment

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

Seems reasonable. If we find ourselves needing to do this more than once we should create an RAII wrapper that does the grabbing and restoring.

}

// Verify we can successfully launch a custom entry point.
TEST_F(WindowsTest, LaunchCustomEntrypoint) {
auto& context = GetContext();
Expand Down