Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
wants to merge 1 commit into from

Conversation

Drupping
Copy link
Contributor

@Drupping Drupping commented Apr 8, 2024

…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.

tests/Makefile.tests Show resolved Hide resolved
@dgibson
Copy link
Owner

dgibson commented Apr 26, 2024

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.

…ll dtb filenames generated by dumptrees to TESTS_TREES_L in Makefile.tests

These two binaries are not produced;
two_roots.dtb and named_root.dtb are instead generated in TESTS_TREES.
Redundant file entries eliminated and Ensures that all dtb filenames
generated by dumptrees are now accounted for in the TEST_TREES, addressing previous omissions

Signed-off-by: Zheng Guangyuan <[email protected]>
@Drupping
Copy link
Contributor Author

  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
Copy link
Owner

dgibson commented Apr 29, 2024

  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
Copy link
Owner

dgibson commented Apr 29, 2024

Merged, thanks.

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

@dgibson dgibson closed this Apr 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants