On Mon, Nov 26, 2018 at 02:56:04PM +0100, Lumir Balhar wrote: > On 11/25/18 11:27 AM, David Gibson wrote: > > On Tue, Nov 20, 2018 at 01:13:48PM +0100, Lumir Balhar wrote: > > > Hello. > > > > > > Could we please finish this? > > Yes, sorry. I did some preliminary changes aiming towards > > simultaneous testing of Python2 and Python3, but then got side tracked > > before completing it. > No worries. > > > I did a rebase of all my patches to the top of current master branch. My py3 > > > branch is available here: https://github.com/frenzymadness/dtc/ and all > > > patches are attached. > > Thanks. > > > > > When I test it with Python 2 or Python 3, everything works. Python version > > > has to be specified by environment variable PYTHON in order to change the > > > default (python2). > > > > > > > $ make check > > > DEP tests/dumptrees.c > > > DEP tests/trees.S > > > DEP tests/testutils.c > > > DEP tests/value-labels.c > > > … > > > Basic sanity check for the FdtRo class ... ok > > > testCreate (__main__.PyLibfdtSwTests) ... ok > > > > > > ---------------------------------------------------------------------- > > > Ran 35 tests in 0.004s > > > > > > OK > > > ********** TEST SUMMARY > > > * Total testcases: 1826 > > > * PASS: 1826 > > > * FAIL: 0 > > > * Bad configuration: 0 > > > * Strange test result: 0 > > > ********** > > > > > > $ PYTHON=python3 make check > > > DEP tests/dumptrees.c > > > DEP tests/trees.S > > > DEP tests/testutils.c > > > DEP tests/value-labels.c > > > … > > > ********** TEST SUMMARY > > > * Total testcases: 1790 > > > * PASS: 1790 > > > * FAIL: 0 > > > * Bad configuration: 0 > > > * Strange test result: 0 > > > ********** > > So, the reason many less tests are run in this case is that the > > Python3 setup.py uses a different name for the extension moule .so. > > That means that the test scripts think there isn't Python support and > > doesn't run those tests. We need to at least fix that before this is > > ready to go. > > That's a great point! In my latest version I am able to run all tests under > both Python 2 and 3. However, Python 3 module has to be renamed after build. > > With Python 2: > > $ make check > … > ********** TEST SUMMARY > * Total testcases: 1880 > * PASS: 1880 > * FAIL: 0 > * Bad configuration: 0 > * Strange test result: 0 > ********** > > WIth Python 3: > > $ PYTHON=python3 make > $ mv pylibfdt/_libfdt{.cpython-37m-x86_64-linux-gnu,}.so > $ PYTHON=python3 make check > … > ********** TEST SUMMARY > * Total testcases: 1880 > * PASS: 1880 > * FAIL: 0 > * Bad configuration: 0 > * Strange test result: 0 > ********** Right. The trick is we need to work out how to either not make the tests depend on the extension module being in that exact file, or automatically move it as part of the build, and we need to do so in a way that's robust across Python revisions. > The question is whether we need to support dual stack with both Pythons in > the same time or not. I'd certainly prefer it, but one at a time is better than nothing. > If yes, we have to find a way how to store both built modules and pick the > right one for tests. Exactly. In fact I think if we don't override the destination setuptools will already put the modules in different locations. The trick is working out how to make our tests link against the right one at runtime. > If no, we can just rename it automatically after every build to the standard > name _libfdt.so Right, but while that's easy to do by hand, working out how to reliably do it automatically isn't quite so obvious. > > What is the preferred way? > > > > > I've merged some bits and pieces from your series, and I've made a few > > other changes to get this closer. I hope this time I can actually get > > to the rest soon. > Great, thanks. I did some more small changes, rebased and I am attaching all > patches again (except the one already merged, of course). > > > >From 064a470664f8ac78162033602e7621b0d207618e Mon Sep 17 00:00:00 2001 > From: Lumir Balhar <lbalhar@xxxxxxxxxx> > Date: Tue, 10 Jul 2018 09:08:07 +0200 > Subject: [PATCH 4/4] pylibfdt: Change how passing tests are recognized > > When some warning appears in test result, "ok" is still > at the end of the line but without three dots. > > Signed-off-by: Lumir Balhar <lbalhar@xxxxxxxxxx> > --- > tests/run_tests.sh | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/tests/run_tests.sh b/tests/run_tests.sh > index ca3fc86..1fa14ff 100755 > --- a/tests/run_tests.sh > +++ b/tests/run_tests.sh > @@ -940,7 +940,7 @@ pylibfdt_tests () { > # and the summary line for total tests (e.g. 'Ran 17 tests in 0.002s'). > # We could add pass + fail to get total tests, but this provides a useful > # sanity check. > - pass_count=$(grep "\.\.\. ok$" $TMP | wc -l) > + pass_count=$(grep "ok$" $TMP | wc -l) > fail_count=$(grep "^ERROR: " $TMP | wc -l) > total_tests=$(sed -n 's/^Ran \([0-9]*\) tests.*$/\1/p' $TMP) > cat $TMP > >From 8a19d132ca0db7e10ec816a39ae660f728df9e9d Mon Sep 17 00:00:00 2001 > From: Petr Viktorin <pviktori@xxxxxxxxxx> > Date: Mon, 9 Jul 2018 14:59:30 +0200 > Subject: [PATCH 3/4] pylibfdt: Test fdt.setprop take bytes on Python 3, add > error handling > > Signed-off-by: Petr Viktorin <pviktori@xxxxxxxxxx> > --- > pylibfdt/libfdt.i | 4 ++++ > tests/pylibfdt_tests.py | 12 +++++++----- > 2 files changed, 11 insertions(+), 5 deletions(-) > > diff --git a/pylibfdt/libfdt.i b/pylibfdt/libfdt.i > index 084bc5b..4f14403 100644 > --- a/pylibfdt/libfdt.i > +++ b/pylibfdt/libfdt.i > @@ -1084,6 +1084,10 @@ typedef uint32_t fdt32_t; > /* typemap used for fdt_setprop() */ > %typemap(in) (const void *val) { > %#if PY_VERSION_HEX >= 0x03000000 > + if (!PyBytes_Check($input)) { > + SWIG_exception_fail(SWIG_TypeError, "bytes expected in method '" "$symname" > + "', argument " "$argnum"" of type '" "$type""'"); > + } > $1 = PyBytes_AsString($input); > %#else > $1 = PyString_AsString($input); /* char *str */ > diff --git a/tests/pylibfdt_tests.py b/tests/pylibfdt_tests.py > index 4761c52..e6c13ff 100644 > --- a/tests/pylibfdt_tests.py > +++ b/tests/pylibfdt_tests.py > @@ -82,6 +82,8 @@ TEST_VALUE64_1 = (TEST_VALUE64_1H << 32) | TEST_VALUE64_1L > PHANDLE_1 = 0x2000 > PHANDLE_2 = 0x2001 > > +TEST_BYTES_1 = b'hello world' > + > TEST_STRING_1 = 'hello world' > TEST_STRING_2 = 'hi world' > TEST_STRING_3 = u'unicode \u01d3' > @@ -443,21 +445,21 @@ class PyLibfdtBasicTests(unittest.TestCase): > def testSetProp(self): > """Test that we can update and create properties""" > node = self.fdt.path_offset('/subnode@1') > - self.fdt.setprop(node, 'compatible', TEST_STRING_1) > - self.assertEquals(TEST_STRING_1, self.fdt.getprop(node, 'compatible')) > + self.fdt.setprop(node, 'compatible', TEST_BYTES_1) > + self.assertEquals(TEST_BYTES_1, self.fdt.getprop(node, 'compatible')) > > # Check that this property is missing, and that we don't have space to > # add it > self.assertEquals(-libfdt.NOTFOUND, > self.fdt.getprop(node, 'missing', QUIET_NOTFOUND)) > self.assertEquals(-libfdt.NOSPACE, > - self.fdt.setprop(node, 'missing', TEST_STRING_1, > + self.fdt.setprop(node, 'missing', TEST_BYTES_1, > quiet=(libfdt.NOSPACE,))) > > # Expand the device tree so we now have room > self.fdt.resize(self.fdt.totalsize() + 50) > - self.fdt.setprop(node, 'missing', TEST_STRING_1) > - self.assertEquals(TEST_STRING_1, self.fdt.getprop(node, 'missing')) > + self.fdt.setprop(node, 'missing', TEST_BYTES_1) > + self.assertEquals(TEST_BYTES_1, self.fdt.getprop(node, 'missing')) > > def testSetPropU32(self): > """Test that we can update and create integer properties""" > >From c96c95413f0432fe1bf36796d5c8027b36ac5b93 Mon Sep 17 00:00:00 2001 > From: Lumir Balhar <lbalhar@xxxxxxxxxx> > Date: Mon, 9 Jul 2018 12:41:13 +0200 > Subject: [PATCH 2/4] pylibfdt: check_err accepts only integer as a first > argument. > > A list passed as an argument to check_err() means that > there is no error code to check and therefore it should > be returned back. > > Signed-off-by: Lumir Balhar <lbalhar@xxxxxxxxxx> > --- > pylibfdt/libfdt.i | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/pylibfdt/libfdt.i b/pylibfdt/libfdt.i > index 6f1f1dc..084bc5b 100644 > --- a/pylibfdt/libfdt.i > +++ b/pylibfdt/libfdt.i > @@ -137,7 +137,7 @@ def check_err(val, quiet=()): > Raises > FdtException if val < 0 > """ > - if val < 0: > + if isinstance(val, int) and val < 0: > if -val not in quiet: > raise FdtException(val) > return val > >From 751a34fee080c9aaca6033aad9501a849a1ab8a7 Mon Sep 17 00:00:00 2001 > From: Lumir Balhar <lbalhar@xxxxxxxxxx> > Date: Mon, 9 Jul 2018 12:40:09 +0200 > Subject: [PATCH 1/4] pylibfdt: Proper handling of bytes/unicode strings and > octal literals > > Signed-off-by: Lumir Balhar <lbalhar@xxxxxxxxxx> > --- > pylibfdt/libfdt.i | 14 +++++++++++--- > tests/pylibfdt_tests.py | 20 ++++++++++---------- > 2 files changed, 21 insertions(+), 13 deletions(-) > > diff --git a/pylibfdt/libfdt.i b/pylibfdt/libfdt.i > index 462b5b0..6f1f1dc 100644 > --- a/pylibfdt/libfdt.i > +++ b/pylibfdt/libfdt.i > @@ -669,7 +669,7 @@ class Fdt(FdtRo): > Raises: > FdtException if no parent found or other error occurs > """ > - val = val.encode('utf-8') + '\0' > + val = val.encode('utf-8') + b'\0' > return check_err(fdt_setprop(self._fdt, nodeoffset, prop_name, > val, len(val)), quiet) > > @@ -1074,12 +1074,20 @@ typedef uint32_t fdt32_t; > if (!$1) > $result = Py_None; > else > - $result = Py_BuildValue("s#", $1, *arg4); > + %#if PY_VERSION_HEX >= 0x03000000 > + $result = Py_BuildValue("y#", $1, *arg4); > + %#else > + $result = Py_BuildValue("s#", $1, *arg4); > + %#endif > } > > /* typemap used for fdt_setprop() */ > %typemap(in) (const void *val) { > - $1 = PyString_AsString($input); /* char *str */ > + %#if PY_VERSION_HEX >= 0x03000000 > + $1 = PyBytes_AsString($input); > + %#else > + $1 = PyString_AsString($input); /* char *str */ > + %#endif > } > > /* typemaps used for fdt_next_node() */ > diff --git a/tests/pylibfdt_tests.py b/tests/pylibfdt_tests.py > index 34c680d..4761c52 100644 > --- a/tests/pylibfdt_tests.py > +++ b/tests/pylibfdt_tests.py > @@ -84,7 +84,7 @@ PHANDLE_2 = 0x2001 > > TEST_STRING_1 = 'hello world' > TEST_STRING_2 = 'hi world' > -TEST_STRING_3 = u'unicode ' + unichr(467) > +TEST_STRING_3 = u'unicode \u01d3' > > > def get_err(err_code): > @@ -107,7 +107,7 @@ def _ReadFdt(fname): > Returns: > Fdt bytearray suitable for passing to libfdt functions > """ > - return libfdt.Fdt(open(fname).read()) > + return libfdt.Fdt(open(fname, mode='rb').read()) > > class PyLibfdtBasicTests(unittest.TestCase): > """Test class for basic pylibfdt access functions > @@ -164,7 +164,7 @@ class PyLibfdtBasicTests(unittest.TestCase): > def testBadFdt(self): > """Check that a filename provided accidentally is not accepted""" > with self.assertRaises(FdtException) as e: > - fdt = libfdt.Fdt('a string') > + fdt = libfdt.Fdt(b'a string') > self.assertEquals(e.exception.err, -libfdt.BADMAGIC) > > def testSubnodeOffset(self): > @@ -239,7 +239,7 @@ class PyLibfdtBasicTests(unittest.TestCase): > poffset = self.fdt.first_property_offset(root) > prop = self.fdt.get_property_by_offset(poffset) > self.assertEquals(prop.name, 'compatible') > - self.assertEquals(prop, 'test_tree1\0') > + self.assertEquals(prop, b'test_tree1\0') > > with self.assertRaises(FdtException) as e: > self.fdt.get_property_by_offset(-2) > @@ -252,7 +252,7 @@ class PyLibfdtBasicTests(unittest.TestCase): > """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(value, b'test_tree1\0') > self.assertEquals(-libfdt.NOTFOUND, self.fdt.getprop(root, 'missing', > QUIET_NOTFOUND)) > > @@ -262,7 +262,7 @@ class PyLibfdtBasicTests(unittest.TestCase): > > node = self.fdt.path_offset('/subnode@1/subsubnode') > value = self.fdt.getprop(node, "compatible") > - self.assertEquals(value, 'subsubnode1\0subsubnode\0') > + self.assertEquals(value, b'subsubnode1\0subsubnode\0') > > def testStrError(self): > """Check that we can get an error string""" > @@ -591,7 +591,7 @@ class PyLibfdtSwTests(unittest.TestCase): > > # Make sure we can read from the tree too > node = sw.path_offset('/subnode@1') > - self.assertEqual('subnode1' + chr(0), sw.getprop(node, 'compatible')) > + self.assertEqual(b'subnode1\0', sw.getprop(node, 'compatible')) > > # Make sure we did at least two resizes > self.assertTrue(len(fdt.as_bytearray()) > FdtSw.INC_SIZE * 2) > @@ -609,15 +609,15 @@ class PyLibfdtRoTests(unittest.TestCase): > > def setUp(self): > """Read in the device tree we use for testing""" > - self.fdt = libfdt.FdtRo(open('test_tree1.dtb').read()) > + self.fdt = libfdt.FdtRo(open('test_tree1.dtb', mode='rb').read()) > > def testAccess(self): > """Basic sanity check for the FdtRo class""" > node = self.fdt.path_offset('/subnode@1') > - self.assertEqual('subnode1' + chr(0), > + self.assertEqual(b'subnode1\0', > self.fdt.getprop(node, 'compatible')) > node = self.fdt.first_subnode(node) > - self.assertEqual('this is a placeholder string\0string2\0', > + self.assertEqual(b'this is a placeholder string\0string2\0', > self.fdt.getprop(node, 'placeholder')) > > -- 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
