| Message ID | 20210413002731.25653-2-laurent.pinchart@ideasonboard.com |
|---|---|
| State | Accepted |
| Commit | c244d07286e4010ee884eeea6f8e67fa5d1ff3cb |
| Headers | show |
| Series |
|
| Related | show |
Hi Laurent, On 13/04/2021 01:27, Laurent Pinchart wrote: > Verify that the begin() and rend() iterators (and their const version) > reference the correct values. The end() and rbegin() iterators can't be > tested the same way as they're not dereferenceable. Can you instead verify that the item preceding the end() or after rbegin() are correct? Or is there no real value in that anyway... But your statement here doesn't seem to match the code. You add tests for begin and cbegin, and rbegin and crbegin. So perhaps you mean you can't test end(), cend(), rend() and crend() because they are not dereferencable.. Presumably here, with the commit message clarified, as I expect the code to have been compile tested: Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com> > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > --- > test/span.cpp | 40 ++++++++++++++++++++++++++++++++-------- > 1 file changed, 32 insertions(+), 8 deletions(-) > > diff --git a/test/span.cpp b/test/span.cpp > index d60b769c9877..ca037c8f02fa 100644 > --- a/test/span.cpp > +++ b/test/span.cpp > @@ -72,12 +72,24 @@ protected: > > staticSpan = Span<int, 4>{ v }; > > - staticSpan.begin(); > - staticSpan.cbegin(); > + if (*staticSpan.begin() != 1) { > + std::cout << "Span<static_extent>::begin() failed" << std::endl; > + return TestFail; > + } > + if (*staticSpan.cbegin() != 1) { > + std::cout << "Span<static_extent>::cbegin() failed" << std::endl; > + return TestFail; > + } > staticSpan.end(); > staticSpan.cend(); > - staticSpan.rbegin(); > - staticSpan.crbegin(); > + if (*staticSpan.rbegin() != 4) { > + std::cout << "Span<static_extent>::rbegin() failed" << std::endl; > + return TestFail; > + } > + if (*staticSpan.crbegin() != 4) { > + std::cout << "Span<static_extent>::crbegin() failed" << std::endl; > + return TestFail; > + } > staticSpan.rend(); > staticSpan.crend(); > > @@ -141,12 +153,24 @@ protected: > > dynamicSpan = Span<int>{ a }; > > - dynamicSpan.begin(); > - dynamicSpan.cbegin(); > + if (*dynamicSpan.begin() != 1) { > + std::cout << "Span<dynamic_extent>::begin() failed" << std::endl; > + return TestFail; > + } > + if (*dynamicSpan.cbegin() != 1) { > + std::cout << "Span<dynamic_extent>::cbegin() failed" << std::endl; > + return TestFail; > + } > dynamicSpan.end(); > dynamicSpan.cend(); > - dynamicSpan.rbegin(); > - dynamicSpan.crbegin(); > + if (*dynamicSpan.rbegin() != 4) { > + std::cout << "Span<dynamic_extent>::rbegin() failed" << std::endl; > + return TestFail; > + } > + if (*dynamicSpan.crbegin() != 4) { > + std::cout << "Span<dynamic_extent>::crbegin() failed" << std::endl; > + return TestFail; > + } > dynamicSpan.rend(); > dynamicSpan.crend(); > >
Hi Kieran, On Tue, Apr 13, 2021 at 09:51:31PM +0100, Kieran Bingham wrote: > On 13/04/2021 01:27, Laurent Pinchart wrote: > > Verify that the begin() and rend() iterators (and their const version) > > reference the correct values. The end() and rbegin() iterators can't be > > tested the same way as they're not dereferenceable. > > Can you instead verify that the item preceding the end() or after > rbegin() are correct? > > Or is there no real value in that anyway... I think more tests for the Span class make sense, including iterating over the whole container, but that's an exercise left to the reader :-) > But your statement here doesn't seem to match the code. > > You add tests for begin and cbegin, and rbegin and crbegin. > > So perhaps you mean you can't test end(), cend(), rend() and crend() > because they are not dereferencable.. Indeed, it should have read Verify that the begin() and rbegin() iterators (and their const version) reference the correct values. The end() and rend() iterators can't be tested the same way as they're not dereferenceable. > Presumably here, with the commit message clarified, as I expect the code > to have been compile tested: > > Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com> > > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > > --- > > test/span.cpp | 40 ++++++++++++++++++++++++++++++++-------- > > 1 file changed, 32 insertions(+), 8 deletions(-) > > > > diff --git a/test/span.cpp b/test/span.cpp > > index d60b769c9877..ca037c8f02fa 100644 > > --- a/test/span.cpp > > +++ b/test/span.cpp > > @@ -72,12 +72,24 @@ protected: > > > > staticSpan = Span<int, 4>{ v }; > > > > - staticSpan.begin(); > > - staticSpan.cbegin(); > > + if (*staticSpan.begin() != 1) { > > + std::cout << "Span<static_extent>::begin() failed" << std::endl; > > + return TestFail; > > + } > > + if (*staticSpan.cbegin() != 1) { > > + std::cout << "Span<static_extent>::cbegin() failed" << std::endl; > > + return TestFail; > > + } > > staticSpan.end(); > > staticSpan.cend(); > > - staticSpan.rbegin(); > > - staticSpan.crbegin(); > > + if (*staticSpan.rbegin() != 4) { > > + std::cout << "Span<static_extent>::rbegin() failed" << std::endl; > > + return TestFail; > > + } > > + if (*staticSpan.crbegin() != 4) { > > + std::cout << "Span<static_extent>::crbegin() failed" << std::endl; > > + return TestFail; > > + } > > staticSpan.rend(); > > staticSpan.crend(); > > > > @@ -141,12 +153,24 @@ protected: > > > > dynamicSpan = Span<int>{ a }; > > > > - dynamicSpan.begin(); > > - dynamicSpan.cbegin(); > > + if (*dynamicSpan.begin() != 1) { > > + std::cout << "Span<dynamic_extent>::begin() failed" << std::endl; > > + return TestFail; > > + } > > + if (*dynamicSpan.cbegin() != 1) { > > + std::cout << "Span<dynamic_extent>::cbegin() failed" << std::endl; > > + return TestFail; > > + } > > dynamicSpan.end(); > > dynamicSpan.cend(); > > - dynamicSpan.rbegin(); > > - dynamicSpan.crbegin(); > > + if (*dynamicSpan.rbegin() != 4) { > > + std::cout << "Span<dynamic_extent>::rbegin() failed" << std::endl; > > + return TestFail; > > + } > > + if (*dynamicSpan.crbegin() != 4) { > > + std::cout << "Span<dynamic_extent>::crbegin() failed" << std::endl; > > + return TestFail; > > + } > > dynamicSpan.rend(); > > dynamicSpan.crend(); > >
diff --git a/test/span.cpp b/test/span.cpp index d60b769c9877..ca037c8f02fa 100644 --- a/test/span.cpp +++ b/test/span.cpp @@ -72,12 +72,24 @@ protected: staticSpan = Span<int, 4>{ v }; - staticSpan.begin(); - staticSpan.cbegin(); + if (*staticSpan.begin() != 1) { + std::cout << "Span<static_extent>::begin() failed" << std::endl; + return TestFail; + } + if (*staticSpan.cbegin() != 1) { + std::cout << "Span<static_extent>::cbegin() failed" << std::endl; + return TestFail; + } staticSpan.end(); staticSpan.cend(); - staticSpan.rbegin(); - staticSpan.crbegin(); + if (*staticSpan.rbegin() != 4) { + std::cout << "Span<static_extent>::rbegin() failed" << std::endl; + return TestFail; + } + if (*staticSpan.crbegin() != 4) { + std::cout << "Span<static_extent>::crbegin() failed" << std::endl; + return TestFail; + } staticSpan.rend(); staticSpan.crend(); @@ -141,12 +153,24 @@ protected: dynamicSpan = Span<int>{ a }; - dynamicSpan.begin(); - dynamicSpan.cbegin(); + if (*dynamicSpan.begin() != 1) { + std::cout << "Span<dynamic_extent>::begin() failed" << std::endl; + return TestFail; + } + if (*dynamicSpan.cbegin() != 1) { + std::cout << "Span<dynamic_extent>::cbegin() failed" << std::endl; + return TestFail; + } dynamicSpan.end(); dynamicSpan.cend(); - dynamicSpan.rbegin(); - dynamicSpan.crbegin(); + if (*dynamicSpan.rbegin() != 4) { + std::cout << "Span<dynamic_extent>::rbegin() failed" << std::endl; + return TestFail; + } + if (*dynamicSpan.crbegin() != 4) { + std::cout << "Span<dynamic_extent>::crbegin() failed" << std::endl; + return TestFail; + } dynamicSpan.rend(); dynamicSpan.crend();
Verify that the begin() and rend() iterators (and their const version) reference the correct values. The end() and rbegin() iterators can't be tested the same way as they're not dereferenceable. Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> --- test/span.cpp | 40 ++++++++++++++++++++++++++++++++-------- 1 file changed, 32 insertions(+), 8 deletions(-)