[libcamera-devel] [PATCH 1/2] test: span: Add tests for begin() and rend()

[Laurent Pinchart]

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 at ideasonboard.com>
> 
> > Signed-off-by: Laurent Pinchart <laurent.pinchart at 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();
> >  

-- 
Regards,

Laurent Pinchart