Hi David,
On 14 February 2017 at 22:44, David Gibson <david@xxxxxxxxxxxxxxxxxxxxx> wrote:
> On Tue, Feb 14, 2017 at 08:51:57PM -0700, Simon Glass wrote:
>> Add a set of tests to cover the functionality in pylibfdt.
>>
>> Signed-off-by: Simon Glass <sjg@xxxxxxxxxxxx>
>> ---
>>
>> 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 | 267 ++++++++++++++++++++++++++++++++++++++++++++++++
>> tests/run_tests.sh | 19 +++-
>> 2 files changed, 285 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.assertEquals(self.fdt.path_offset('/subnode@1'), 124)
>
> This test is potentially fragile. Eventually it would be nice to be
> able to run the Python tests expecting test_tree1 on any of the copies
> of test_tree1 we generate. Those are required to be semantically
> identicaly (including node/property order) to test_tree1.dtb.
> However, some versions won't preserve exact offsets - for example
> there's a sequence of tests where we insert additional nops in the
> encoding to test handling of that. That's why tests/path_offset.c,
> for example, checks the behaviour of path_offset() against
> subnode_offset() and knowing what property and node names are supposed
> to be present, rather than against explicit known offsets.I'm
Yes it is fragile, will check it's >0 which should be safe.
Re the tests, I feel we should try to avoid testing all the same
things as the C code, when we could just test the interface. But it
might be easier just to duplicate the tests you as say.
>
>> + with self.assertRaisesRegexp(FdtException, get_err(libfdt.NOTFOUND)):
>> + self.fdt.path_offset('/wibble')
>> + 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.NOTFOUND)):
>> + self.fdt.next_property_offset(108)
>
> Same issue here.
OK, I can just drop this one.
>
>> + 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)
>> +
>> + node = self.fdt.path_offset('/subnode@1/ss1')
>> + self.assertEquals(self.fdt.first_property_offset(node, QUIET_NOTFOUND),
>> + -libfdt.NOTFOUND)
>> + with self.assertRaisesRegexp(FdtException, get_err(libfdt.NOTFOUND)):
>> + self.fdt.first_property_offset(node)
>> +
>> + def testGetName(self):
>> + """Check that we can get the name of a node"""
>> + self.assertEquals(self.fdt.get_name(0), '')
>> + node = self.fdt.path_offset('/subnode@1/subsubnode')
>> + self.assertEquals(self.fdt.get_name(node), 'subsubnode')
>> +
>> + with self.assertRaisesRegexp(FdtException, get_err(libfdt.BADOFFSET)):
>> + self.fdt.get_name(-2)
>> +
>> + def testGetPropertyByOffset(self):
>> + """Check that we can read the name and contents of a property"""
>> + root = self.fdt.path_offset('/')
>
> No point to this - offset of / is always 0. If you want to test that
> happens, make it a separate testcase.
I already have it above so will drop this.
>
>> + poffset = self.fdt.first_property_offset(root)
>> + prop = self.fdt.get_property_by_offset(poffset)
>> + self.assertEquals(prop.name, 'compatible')
>> + self.assertEquals(prop.value, 'test_tree1\0')
>> +
>> + with self.assertRaisesRegexp(FdtException, get_err(libfdt.BADOFFSET)):
>> + self.fdt.get_property_by_offset(-2)
>> + self.assertEquals(
>> + -libfdt.BADOFFSET,
>> + self.fdt.get_property_by_offset(-2, [libfdt.BADOFFSET]))
>> +
>> + def testGetProp(self):
>> + """Check that we can read the contents of a property by name"""
>> + root = self.fdt.path_offset('/')
>> + value = self.fdt.getprop(root, "compatible")
>> + self.assertEquals(value, 'test_tree1\0')
>> + self.assertEquals(-libfdt.NOTFOUND, self.fdt.getprop(root, 'missing',
>> + QUIET_NOTFOUND))
>
> For testing, it might be useful to add a special value you can set the
> quiet parameter to to make all errors quiet.
Isn't that what I did?
Regards,
Simon
--
To unsubscribe from this list: send the line "unsubscribe devicetree-compiler" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
