Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix compatibility problems with a pre-Cohen save #3070

Merged
merged 8 commits into from
Jul 18, 2021

Conversation

pleroy
Copy link
Member

@pleroy pleroy commented Jul 17, 2021

While I am at it I am reviving the compatibility test to make sure that this doesn't regress. The test save is relatively small at 1 828 824 bytes.

Fix #3039.

const char preferred_compressor[] = "gipfeli";
const char preferred_encoder[] = "base64";

class InterfaceCompatibilityTest : public testing::Test {
Copy link
Member

Choose a reason for hiding this comment

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

Let us keep that in PluginCompatibilityTest; while this test is interface-only, we may want to inspect saves in other tests, e.g., https://github.com/eggrobin/Principia/blob/saves/ksp_plugin_test/plugin_compatibility_test.cpp.

Copy link
Member Author

Choose a reason for hiding this comment

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

Done.

}
EXPECT_THAT(plugin, NotNull());

// Write that plugin back to another file with the preferred format.
Copy link
Member

@eggrobin eggrobin Jul 17, 2021

Choose a reason for hiding this comment

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

Split the reading and writing into separate functions.

Sorry, something went wrong.

Copy link
Member Author

Choose a reason for hiding this comment

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

Done.

Sorry, something went wrong.

: std::vector<std::string>{};
CHECK(!lines.empty());

LOG(ERROR) << "Deserialization starting";
Copy link
Member

Choose a reason for hiding this comment

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

LOG(INFO) and google::LogToStdErr() in the constructor.

Copy link
Member Author

Choose a reason for hiding this comment

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

Nope. When I do that the output gets swamped with messages emitted from various places within the plugin (and its subobjects). I want to be able to the progress of the test, not random crud unrelated to the test.

@eggrobin eggrobin added the LGTM label Jul 17, 2021
@pleroy pleroy merged commit 30c1060 into mockingbirdnest:master Jul 18, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Green & Grassmann OK but Gröbner crashes with old RetroRendezvous save
2 participants