search

dgibson / dtc

Device Tree Compiler
235 stars 134 forks source link

tests: Remove two_roots and named_root from LIBTREE_TESTS_L in Makefi… #133

Closed Drupping closed 7 months ago

Drupping commented 7 months ago

…le.tests.

These two binaries are not produced; two_roots.dtb and named_root.dtb are instead generated in TESTS_TREES. Redundant file entries eliminated.

dgibson commented 7 months ago

So, there are actually three overlapping problems here. Two are relatively simple:

  1. LIBTREE_TESTS is supposed to list the test binaries which need to be linked against trees.S and there aren't test binaries for named_root and two_roots
  2. TREE_TESTS should list all the dtbs which are generated by dumptrees from trees.S - currently they're all missing except for test_tree1.dtb. Not just named_root and two_roots but all the others as well.

If you could update your patch to fix both of those, that would be appreciated.

The more complex problem is that we don't have any tests using the named_root and two_roots examples. I'm not sure if they were just never written, or if they were written, but I forgot to commit them at some point. Either way, I'm pretty sure they're lost now. If we wrote testcases for these they'd most likely have the same names, and would be added again to LIBTREE_TESTS.

Drupping commented 7 months ago
  1. removing two_roots and named_root from LIBTREE_TESTS_L was indeed the intention behind my initial commit.
  2. I have already updated the patch to include all the dtbs generated by dumptrees from trees.S.
  3. I suspect there was never an initial requirement to write tests for named_root and two_roots, given that testing for these scenarios seems to be covered in the commits with IDs starting with a2def54 and 4ca61f8. In these commits, two_roots and named_root were added to LIBTREE_TESTS_L but, this inclusion was redundant.
dgibson commented 7 months ago
  1. removing two_roots and named_root from LIBTREE_TESTS_L was indeed the intention behind my initial commit.

Right, that's what I thought.

2. I have already updated the patch to include all the dtbs generated by dumptrees from trees.S.

Thank you.

3. I suspect there was never an initial requirement to write tests for named_root and two_roots, given that testing for these scenarios seems to be covered in the commits with IDs starting with [a2def54](https://github.com/dgibson/dtc/commit/a2def5479950d066dcf829b8c1a22874d0aea9a4) and [4ca61f8](https://github.com/dgibson/dtc/commit/4ca61f84dc210ae78376d992c1ce6ebe40ecb5be). In these commits, two_roots and named_root were added to LIBTREE_TESTS_L but, this inclusion was redundant.

Ah, good point. I missed the fact that those dtbs were used in the tests for the fdt_check function.

dgibson commented 7 months ago

Merged, thanks.

There's an annoying test failure on windows, but it appears to be unrelated to this patch.