Make it build and pass unittests on my machine #96
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Allows using GMock command line options.
Necessary for GTest and GMock update.
Fixes random segfaults in RLMachineTest.SerializationOfSavepointValues.
- Operations `recMulti` and `grpMulti` each have two variants with different opcodes for different types of arguments. There's no way to tell these variants apart using just op name and overload index. - RLModule allows us to iterate over all operations from the module. The order of iteration is undefined because it uses std::unordered_map under the hood. - TestMachine fills an internal operations registry by iterating over module operations and adding them to a map. Map keys are pairs (op name, overload index). If there are duplicate keys (as it is with recMulti), we just overwrite whatever we recorded earlier. This is clearly wrong. - The solution here is to add a crutch that ignores unneeded operation variants in testing code. There's now also a check for key dupes. - There was an ifdef for using `std::map` instead of `std::unordered_map` in RLModule for Apple builds to fix some failing test. I presume it was the same test that gets fixed here so that ifdef also got removed.
|
(It will be a few more days until I can review this patch. Sorry.) |
|
Read everything. Thank you. I can't actually build anything right now (I don't have an ubuntu machine anymore and my mac isn't set up for development), but everything here makes sense so I'm just going to go ahead and merge this. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There were some minor compilation errors and failing unittests.
One test was most likely failing because of google/googletest#705, I've updated gtest and gmock, now it's green.
Another couple of tests were failing because of the undefined iteration order for unordered_map. Also there was a hack for Apple builds that was probably supposed to fix that test by replacing the container with regular map. I've moved the hack to testing code and made it more reliable.
(This PR can be splitted, the commits are more or less independent.)