[Lldb-commits] [PATCH] D49415: Add unit tests for VMRange

lemo added subscribers: teemperor, lemo. lemo added a comment. The new test cases did catch a real bug: [ RUN ] VMRange.CollectionContains llvm/src/tools/lldb/unittests/Utility/VMRangeTest.cpp:146: Failure Value of: VMRange::ContainsRange(collection, VMRange(0x100, 0x104)) Actual: false

Re: [Lldb-commits] [PATCH] D49415: Add unit tests for VMRange

The new test cases did catch a real bug: [ RUN ] VMRange.CollectionContains llvm/src/tools/lldb/unittests/Utility/VMRangeTest.cpp:146: Failure Value of: VMRange::ContainsRange(collection, VMRange(0x100, 0x104)) Actual: false Expected: true class RangeInRangeUnaryPredicate { public:

[Lldb-commits] [PATCH] D49415: Add unit tests for VMRange

This revision was not accepted when it landed; it landed in state "Needs Review". This revision was automatically updated to reflect the committed changes. Closed by commit rL337873: Add unit tests for VMRange (authored by teemperor, committed by ). Herald added a subscriber: llvm-commits.

[Lldb-commits] [PATCH] D49415: Add unit tests for VMRange

teemperor added a comment. Merging this in as Davide suggested. https://reviews.llvm.org/D49415 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D49415: Add unit tests for VMRange

teemperor updated this revision to Diff 157172. teemperor added a comment. - Applied Greg's requested changes (thank you very much). https://reviews.llvm.org/D49415 Files: unittests/Utility/CMakeLists.txt unittests/Utility/VMRangeTest.cpp Index: unittests/Utility/VMRangeTest.cpp

[Lldb-commits] [PATCH] D49415: Add unit tests for VMRange

labath added a comment. In https://reviews.llvm.org/D49415#1165179, @teemperor wrote: > - Added a PrintTo function as otherwise the gtest comparison macros won't > compile. Sorry, I did not anticipate that. It looks like the `iterator` typedef inside the VMRange is confusing gtest's

[Lldb-commits] [PATCH] D49415: Add unit tests for VMRange

clayborg added inline comments. Comment at: unittests/Utility/VMRangeTest.cpp:42 + VMRange range1(0x100, 0x200); + VMRange range2(0x100, 0x200); + EXPECT_EQ(range1, range2); Seems like a few more edge cases might be good: ``` EXPECT_NE(range1, VMRange(0x100,

[Lldb-commits] [PATCH] D49415: Add unit tests for VMRange

teemperor updated this revision to Diff 155899. teemperor added a comment. - ASSERT_ -> EXPECT_ - Now using gtest's comparison macros where possible. - Added a PrintTo function as otherwise the gtest comparison macros won't compile. https://reviews.llvm.org/D49415 Files:

[Lldb-commits] [PATCH] D49415: Add unit tests for VMRange

labath added a comment. The tests seem fine, but as a matter of style I would suggest two changes: - replace `ASSERT_XXX` with `EXPECT_XXX`: EXPECT macros will not terminate the test, so you'll be able to see additional failures, which is helpful in pinpointing the problem. ASSERT is good for

[Lldb-commits] [PATCH] D49415: Add unit tests for VMRange

teemperor created this revision. Herald added a subscriber: mgorny. https://reviews.llvm.org/D49415 Files: unittests/Utility/CMakeLists.txt unittests/Utility/VMRangeTest.cpp Index: unittests/Utility/VMRangeTest.cpp === ---