Re: [PATCH v7 2/5] Add tests for pylibfdt

[David Gibson <david-xT8FGy+AXnRB3Ne2BGzF6laj5H9X9Tb+@public.gmane.org>]

[-- Attachment #1: Type: text/plain, Size: 4091 bytes --]

On Tue, Feb 28, 2017 at 10:40:32PM -0700, Simon Glass wrote:
> Hi David,
> 
> On 23 February 2017 at 19:52, David Gibson <david-xT8FGy+AXnRB3Ne2BGzF6laj5H9X9Tb+@public.gmane.org> wrote:
> > On Tue, Feb 21, 2017 at 09:33:37PM -0700, Simon Glass wrote:
> >> Add a set of tests to cover the functionality in pylibfdt.
> >>
> >> Signed-off-by: Simon Glass <sjg-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
> >> ---
> >>
> >> Changes in v7:
> >> - Add a test for QUIET_ALL
> >>
> >> Changes in v6:
> >> - Adjust tests to avoid checking a hard-coded offset
> >> - Use 0 instead of self.fdt.path_offset('/')
> >> - Adjust the totalsize() test to compare against the file size
> >> - Adjust test result processing to avoid using awk
> >>
> >> Changes in v5:
> >> - Adjust tests to match new swig bindings
> >>
> >> Changes in v4:
> >> - Drop tests that are no-longer applicable
> >> - Add a get for getprop()
> >>
> >> Changes in v3:
> >> - Add some more tests
> >>
> >> Changes in v2:
> >> - Update tests for new pylibfdt
> >> - Add a few more tests to increase coverage
> >>
> >>  tests/pylibfdt_tests.py | 275 ++++++++++++++++++++++++++++++++++++++++++++++++
> >>  tests/run_tests.sh      |  16 ++-
> >>  2 files changed, 290 insertions(+), 1 deletion(-)
> >>  create mode 100644 tests/pylibfdt_tests.py
> >>
> 
> >> +    def testPathOffset(self):
> >> +        """Check that we can find the offset of a node"""
> >> +        self.assertEquals(self.fdt.path_offset('/'), 0)
> >> +        self.assertTrue(self.fdt.path_offset('/subnode@1') > 0)
> >> +        with self.assertRaisesRegexp(FdtException, get_err(libfdt.NOTFOUND)):
> >> +            self.fdt.path_offset('/wibble')
> >
> > Checking the error string by regexp seems a bit indirect.  Wouldn't it
> > be better to add new helper that checks explicitly for an FdtException
> > with the right fdt error code embedded in it?
> 
> I'm not sure how to do a helper simply since there is a context
> manager involved. I'll use the normal method of checking the exception
> afterwards.

Ah.. yeah.. Python with statements always confuse me.

> 
> >
> >> +        self.assertEquals(self.fdt.path_offset('/wibble', QUIET_NOTFOUND),
> >> +                          -libfdt.NOTFOUND)
> >> +
> >> +    def testPropertyOffset(self):
> >> +        """Walk through all the properties in the root node"""
> >> +        self.assertEquals(self.fdt.first_property_offset(0), ROOT_PROPS[0])
> >> +        for pos in range(len(ROOT_PROPS) - 1):
> >> +            self.assertEquals(self.fdt.next_property_offset(ROOT_PROPS[pos]),
> >> +                              ROOT_PROPS[pos + 1])
> >> +        self.assertEquals(self.fdt.next_property_offset(ROOT_PROPS[-1],
> >> +                                                        QUIET_NOTFOUND),
> >> +                          -libfdt.NOTFOUND)
> >> +
> >> +    def testPropertyOffsetExceptions(self):
> >> +        """Check that exceptions are raised as expected"""
> >> +        with self.assertRaisesRegexp(FdtException, get_err(libfdt.BADOFFSET)):
> >> +            self.fdt.next_property_offset(107)
> >> +        with self.assertRaisesRegexp(FdtException, get_err(libfdt.BADOFFSET)):
> >> +            self.fdt.first_property_offset(107, QUIET_NOTFOUND)
> >> +        with self.assertRaisesRegexp(FdtException, get_err(libfdt.BADOFFSET)):
> >> +            self.fdt.next_property_offset(107, QUIET_NOTFOUND)
> >
> > Isn't this a dupe of the previous statement?  This also looks like a
> > case where using QUIET_ALL could make it neater.
> 
> All three statements are slightly different.
> 
> The intent here is to make sure that quietening one error doesn't mask another.

Ah.. ok.  Maybe a comment to that effect - from the test name, I
thought it was just about the underlying function returning the right
error codes.

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]