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

[3.2] simplify deep_mine_tests by replacing complicated merge and match with straight-forward file copying and comparison #593

Merged
merged 5 commits into from
Jul 12, 2022

Conversation

linh2931
Copy link
Member

@linh2931 linh2931 commented Jul 3, 2022

Resolve #384

#383 fixed non-determinism issues with deep-mind logger by zeroing out the elapsed fields within a copy of the transaction trace before writing it out to the logs in tests. I have confirmed that fix produced deterministic logs. The complicated merge and match code in deep_mine_tests are replaced with simple file copying and comparison.

…h straight-forward file saving and comparison, made possible by PR #383's fix of non-determinism of deep-mind logger
@linh2931 linh2931 requested review from arhag and spoonincode July 3, 2022 15:22
@linh2931 linh2931 self-assigned this Jul 3, 2022
{
BOOST_TEST(false, "Expected end of file at line " << i);
BOOST_TEST(false, "Expected end of file1 at line1 " << i);
Copy link
Contributor

Choose a reason for hiding this comment

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

this is a string, not a variable name.

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks @swatanabe. I modified the wording.

Comment on lines 59 to 60
while(std::getline(input_file, s)) {
output_file << s << "\n";
Copy link
Contributor

Choose a reason for hiding this comment

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

Can use output_file << input_file.rdbuf() or just fc::copy and get rid of the streams entirely.

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks for the suggestion. It further simplifies the code. I changed to use fc:rename as fc:copy does not allow destination to exist before copying.

@linh2931 linh2931 merged commit 685d5f1 into main Jul 12, 2022
@linh2931 linh2931 deleted the simplify_deep_mind_log branch July 12, 2022 01:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Revisit logging approach in deep_mind_tests
3 participants