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@xxxxxxxxxxxxxxxxxxxxx> 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@xxxxxxxxxxxx>
> >> ---
> >>
> >> 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:
signature.asc
Description: PGP signature
