[Lldb-commits] [PATCH] D56322: [Reproducers] SBReproducer framework

[Pavel Labath via Phabricator via lldb-commits]

labath added inline comments.


================
Comment at: unittests/Utility/ReproducerInstrumentationTest.cpp:399-404
+  EXPECT_EQ(foo.g_a, 100);
+  EXPECT_EQ(foo.g_b, 200);
+  EXPECT_TRUE(Equals(foo.g_c, c));
+  EXPECT_EQ(foo.g_d, "bar");
+  EXPECT_TRUE(Equals(foo.g_e, e));
+  EXPECT_EQ(foo.g_f, true);
----------------
labath wrote:
> access variables via the class (InstrumentedFoo::g_a) to make it clear that they are static.
Actually, what do you think about this pattern:
- have the variables be member variables
- add a `Validate` method to the InstrumentedFoo class. This method would also be instrumented, and it would check that the members are set to the correct value
- call the method during recording and it will be automatically called during replay. Then all you need to do in the test is check that `Validate` was called.

I am hoping that this will make things more manageable once you have multiple instrumented objects floating around in the test. Also, this would implicitly test that the replay framework calls the member functions on the right object (e.g., right now this would succeed if the framework just created a fresh new object for each call).


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D56322/new/

https://reviews.llvm.org/D56322