| Message ID | 20230929160654.3475279-1-sean.anderson@seco.com |
|---|---|
| State | Accepted |
| Commit | 3f876cb7c57511174d1b6a3e089443ccbaf236ec |
| Delegated to: | Tom Rini |
| Headers | show |
| Series | test: Fix SPL tests not being run | expand |
On 9/29/23 12:06, Sean Anderson wrote: > SPL doesn't have OF_LIVE enabled, so we can only run tests with a flat > tree. Don't skip them even if they don't use the devicetree. > > Fixes: 6ec5178c0ef ("test: Skip flat-tree tests if devicetree is not used") > Signed-off-by: Sean Anderson <sean.anderson@seco.com> > --- > > test/test-main.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/test/test-main.c b/test/test-main.c > index 778bf0a18a0..edb20bc4b9c 100644 > --- a/test/test-main.c > +++ b/test/test-main.c > @@ -476,7 +476,8 @@ static int ut_run_test_live_flat(struct unit_test_state *uts, > * (for sandbox we handle this by copying the tree, but not for other > * boards) > */ > - if ((test->flags & UT_TESTF_SCAN_FDT) && > + if ((!CONFIG_IS_ENABLED(OF_LIVE) || > + (test->flags & UT_TESTF_SCAN_FDT)) && > !(test->flags & UT_TESTF_LIVE_TREE) && > (CONFIG_IS_ENABLED(OFNODE_MULTI_TREE) || > !(test->flags & UT_TESTF_OTHER_FDT)) && Upon further review, do we even need 6ec5178c0ef in the first place? ut_test_run_on_flattree looks like it's trying to do the same thing. --Sean
Hi Sean, On Fri, 29 Sept 2023 at 10:12, Sean Anderson <sean.anderson@seco.com> wrote: > > On 9/29/23 12:06, Sean Anderson wrote: > > SPL doesn't have OF_LIVE enabled, so we can only run tests with a flat > > tree. Don't skip them even if they don't use the devicetree. > > > > Fixes: 6ec5178c0ef ("test: Skip flat-tree tests if devicetree is not used") > > Signed-off-by: Sean Anderson <sean.anderson@seco.com> > > --- > > > > test/test-main.c | 3 ++- > > 1 file changed, 2 insertions(+), 1 deletion(-) > > > > diff --git a/test/test-main.c b/test/test-main.c > > index 778bf0a18a0..edb20bc4b9c 100644 > > --- a/test/test-main.c > > +++ b/test/test-main.c > > @@ -476,7 +476,8 @@ static int ut_run_test_live_flat(struct unit_test_state *uts, > > * (for sandbox we handle this by copying the tree, but not for other > > * boards) > > */ > > - if ((test->flags & UT_TESTF_SCAN_FDT) && > > + if ((!CONFIG_IS_ENABLED(OF_LIVE) || > > + (test->flags & UT_TESTF_SCAN_FDT)) && > > !(test->flags & UT_TESTF_LIVE_TREE) && > > (CONFIG_IS_ENABLED(OFNODE_MULTI_TREE) || > > !(test->flags & UT_TESTF_OTHER_FDT)) && > > Upon further review, do we even need 6ec5178c0ef in the first place? > ut_test_run_on_flattree looks like it's trying to do the same thing. Well one problem is that many tests are not run at all unless OF_LIVE is enabled. The code as is is assuming that OF_LIVE is active. On boards where OF_LIVE is not active, many tests won't run at all unless they are marked with UT_TESTF_SCAN_FDT. So I think that UT_TESTF_SCAN_FDT line needs to be removed. Regards, Simon
On 10/1/23 15:36, Simon Glass wrote: > Hi Sean, > > On Fri, 29 Sept 2023 at 10:12, Sean Anderson <sean.anderson@seco.com> wrote: >> >> On 9/29/23 12:06, Sean Anderson wrote: >> > SPL doesn't have OF_LIVE enabled, so we can only run tests with a flat >> > tree. Don't skip them even if they don't use the devicetree. >> > >> > Fixes: 6ec5178c0ef ("test: Skip flat-tree tests if devicetree is not used") >> > Signed-off-by: Sean Anderson <sean.anderson@seco.com> >> > --- >> > >> > test/test-main.c | 3 ++- >> > 1 file changed, 2 insertions(+), 1 deletion(-) >> > >> > diff --git a/test/test-main.c b/test/test-main.c >> > index 778bf0a18a0..edb20bc4b9c 100644 >> > --- a/test/test-main.c >> > +++ b/test/test-main.c >> > @@ -476,7 +476,8 @@ static int ut_run_test_live_flat(struct unit_test_state *uts, >> > * (for sandbox we handle this by copying the tree, but not for other >> > * boards) >> > */ >> > - if ((test->flags & UT_TESTF_SCAN_FDT) && >> > + if ((!CONFIG_IS_ENABLED(OF_LIVE) || >> > + (test->flags & UT_TESTF_SCAN_FDT)) && >> > !(test->flags & UT_TESTF_LIVE_TREE) && >> > (CONFIG_IS_ENABLED(OFNODE_MULTI_TREE) || >> > !(test->flags & UT_TESTF_OTHER_FDT)) && >> >> Upon further review, do we even need 6ec5178c0ef in the first place? >> ut_test_run_on_flattree looks like it's trying to do the same thing. > > Well one problem is that many tests are not run at all unless OF_LIVE > is enabled. The code as is is assuming that OF_LIVE is active. > > On boards where OF_LIVE is not active, many tests won't run at all > unless they are marked with UT_TESTF_SCAN_FDT. > > So I think that UT_TESTF_SCAN_FDT line needs to be removed. OK, so to clarify, since 6ec5178c0ef added that UT_TESTF_SCAN_FDT, you would like to revert that commit? --Sean
Hi Sean, On Mon, 2 Oct 2023 at 08:38, Sean Anderson <sean.anderson@seco.com> wrote: > > On 10/1/23 15:36, Simon Glass wrote: > > Hi Sean, > > > > On Fri, 29 Sept 2023 at 10:12, Sean Anderson <sean.anderson@seco.com> wrote: > >> > >> On 9/29/23 12:06, Sean Anderson wrote: > >> > SPL doesn't have OF_LIVE enabled, so we can only run tests with a flat > >> > tree. Don't skip them even if they don't use the devicetree. > >> > > >> > Fixes: 6ec5178c0ef ("test: Skip flat-tree tests if devicetree is not used") > >> > Signed-off-by: Sean Anderson <sean.anderson@seco.com> > >> > --- > >> > > >> > test/test-main.c | 3 ++- > >> > 1 file changed, 2 insertions(+), 1 deletion(-) > >> > > >> > diff --git a/test/test-main.c b/test/test-main.c > >> > index 778bf0a18a0..edb20bc4b9c 100644 > >> > --- a/test/test-main.c > >> > +++ b/test/test-main.c > >> > @@ -476,7 +476,8 @@ static int ut_run_test_live_flat(struct unit_test_state *uts, > >> > * (for sandbox we handle this by copying the tree, but not for other > >> > * boards) > >> > */ > >> > - if ((test->flags & UT_TESTF_SCAN_FDT) && > >> > + if ((!CONFIG_IS_ENABLED(OF_LIVE) || > >> > + (test->flags & UT_TESTF_SCAN_FDT)) && > >> > !(test->flags & UT_TESTF_LIVE_TREE) && > >> > (CONFIG_IS_ENABLED(OFNODE_MULTI_TREE) || > >> > !(test->flags & UT_TESTF_OTHER_FDT)) && > >> > >> Upon further review, do we even need 6ec5178c0ef in the first place? > >> ut_test_run_on_flattree looks like it's trying to do the same thing. > > > > Well one problem is that many tests are not run at all unless OF_LIVE > > is enabled. The code as is is assuming that OF_LIVE is active. > > > > On boards where OF_LIVE is not active, many tests won't run at all > > unless they are marked with UT_TESTF_SCAN_FDT. > > > > So I think that UT_TESTF_SCAN_FDT line needs to be removed. > > OK, so to clarify, since 6ec5178c0ef added that UT_TESTF_SCAN_FDT, you would like to > revert that commit? Yes, I think that will work...but just check that tests without the UT_TESTF_SCAN_FDT flag don't then run twice with sandbox. There was perhaps something else wrong at the time. Regards, Simon
On 10/2/23 14:56, Simon Glass wrote: > Hi Sean, > > On Mon, 2 Oct 2023 at 08:38, Sean Anderson <sean.anderson@seco.com> wrote: >> >> On 10/1/23 15:36, Simon Glass wrote: >> > Hi Sean, >> > >> > On Fri, 29 Sept 2023 at 10:12, Sean Anderson <sean.anderson@seco.com> wrote: >> >> >> >> On 9/29/23 12:06, Sean Anderson wrote: >> >> > SPL doesn't have OF_LIVE enabled, so we can only run tests with a flat >> >> > tree. Don't skip them even if they don't use the devicetree. >> >> > >> >> > Fixes: 6ec5178c0ef ("test: Skip flat-tree tests if devicetree is not used") >> >> > Signed-off-by: Sean Anderson <sean.anderson@seco.com> >> >> > --- >> >> > >> >> > test/test-main.c | 3 ++- >> >> > 1 file changed, 2 insertions(+), 1 deletion(-) >> >> > >> >> > diff --git a/test/test-main.c b/test/test-main.c >> >> > index 778bf0a18a0..edb20bc4b9c 100644 >> >> > --- a/test/test-main.c >> >> > +++ b/test/test-main.c >> >> > @@ -476,7 +476,8 @@ static int ut_run_test_live_flat(struct unit_test_state *uts, >> >> > * (for sandbox we handle this by copying the tree, but not for other >> >> > * boards) >> >> > */ >> >> > - if ((test->flags & UT_TESTF_SCAN_FDT) && >> >> > + if ((!CONFIG_IS_ENABLED(OF_LIVE) || >> >> > + (test->flags & UT_TESTF_SCAN_FDT)) && >> >> > !(test->flags & UT_TESTF_LIVE_TREE) && >> >> > (CONFIG_IS_ENABLED(OFNODE_MULTI_TREE) || >> >> > !(test->flags & UT_TESTF_OTHER_FDT)) && >> >> >> >> Upon further review, do we even need 6ec5178c0ef in the first place? >> >> ut_test_run_on_flattree looks like it's trying to do the same thing. >> > >> > Well one problem is that many tests are not run at all unless OF_LIVE >> > is enabled. The code as is is assuming that OF_LIVE is active. >> > >> > On boards where OF_LIVE is not active, many tests won't run at all >> > unless they are marked with UT_TESTF_SCAN_FDT. >> > >> > So I think that UT_TESTF_SCAN_FDT line needs to be removed. >> >> OK, so to clarify, since 6ec5178c0ef added that UT_TESTF_SCAN_FDT, you would like to >> revert that commit? > > Yes, I think that will work...but just check that tests without the > UT_TESTF_SCAN_FDT flag don't then run twice with sandbox. There was > perhaps something else wrong at the time. Actually, upon further review, I think that the above patch is correct. A revert would cause tests with UT_TESTF_DM but without UT_TESTF_SCAN_FDT to run twice. --Sean
On Thu, 5 Oct 2023 at 16:24, Sean Anderson <sean.anderson@seco.com> wrote: > > On 10/2/23 14:56, Simon Glass wrote: > > Hi Sean, > > > > On Mon, 2 Oct 2023 at 08:38, Sean Anderson <sean.anderson@seco.com> wrote: > >> > >> On 10/1/23 15:36, Simon Glass wrote: > >> > Hi Sean, > >> > > >> > On Fri, 29 Sept 2023 at 10:12, Sean Anderson <sean.anderson@seco.com> wrote: > >> >> > >> >> On 9/29/23 12:06, Sean Anderson wrote: > >> >> > SPL doesn't have OF_LIVE enabled, so we can only run tests with a flat > >> >> > tree. Don't skip them even if they don't use the devicetree. > >> >> > > >> >> > Fixes: 6ec5178c0ef ("test: Skip flat-tree tests if devicetree is not used") > >> >> > Signed-off-by: Sean Anderson <sean.anderson@seco.com> > >> >> > --- > >> >> > > >> >> > test/test-main.c | 3 ++- > >> >> > 1 file changed, 2 insertions(+), 1 deletion(-) > >> >> > > >> >> > diff --git a/test/test-main.c b/test/test-main.c > >> >> > index 778bf0a18a0..edb20bc4b9c 100644 > >> >> > --- a/test/test-main.c > >> >> > +++ b/test/test-main.c > >> >> > @@ -476,7 +476,8 @@ static int ut_run_test_live_flat(struct unit_test_state *uts, > >> >> > * (for sandbox we handle this by copying the tree, but not for other > >> >> > * boards) > >> >> > */ > >> >> > - if ((test->flags & UT_TESTF_SCAN_FDT) && > >> >> > + if ((!CONFIG_IS_ENABLED(OF_LIVE) || > >> >> > + (test->flags & UT_TESTF_SCAN_FDT)) && > >> >> > !(test->flags & UT_TESTF_LIVE_TREE) && > >> >> > (CONFIG_IS_ENABLED(OFNODE_MULTI_TREE) || > >> >> > !(test->flags & UT_TESTF_OTHER_FDT)) && > >> >> > >> >> Upon further review, do we even need 6ec5178c0ef in the first place? > >> >> ut_test_run_on_flattree looks like it's trying to do the same thing. > >> > > >> > Well one problem is that many tests are not run at all unless OF_LIVE > >> > is enabled. The code as is is assuming that OF_LIVE is active. > >> > > >> > On boards where OF_LIVE is not active, many tests won't run at all > >> > unless they are marked with UT_TESTF_SCAN_FDT. > >> > > >> > So I think that UT_TESTF_SCAN_FDT line needs to be removed. > >> > >> OK, so to clarify, since 6ec5178c0ef added that UT_TESTF_SCAN_FDT, you would like to > >> revert that commit? > > > > Yes, I think that will work...but just check that tests without the > > UT_TESTF_SCAN_FDT flag don't then run twice with sandbox. There was > > perhaps something else wrong at the time. > > Actually, upon further review, I think that the above patch is correct. A revert would > cause tests with UT_TESTF_DM but without UT_TESTF_SCAN_FDT to run twice. > Thanks Reviewed-by: Simon Glass <sjg@chromium.org>
On Fri, Sep 29, 2023 at 12:06:54PM -0400, Sean Anderson wrote: > SPL doesn't have OF_LIVE enabled, so we can only run tests with a flat > tree. Don't skip them even if they don't use the devicetree. > > Fixes: 6ec5178c0ef ("test: Skip flat-tree tests if devicetree is not used") > Signed-off-by: Sean Anderson <sean.anderson@seco.com> > Reviewed-by: Simon Glass <sjg@chromium.org> Applied to u-boot/master, thanks!
diff --git a/test/test-main.c b/test/test-main.c index 778bf0a18a0..edb20bc4b9c 100644 --- a/test/test-main.c +++ b/test/test-main.c @@ -476,7 +476,8 @@ static int ut_run_test_live_flat(struct unit_test_state *uts, * (for sandbox we handle this by copying the tree, but not for other * boards) */ - if ((test->flags & UT_TESTF_SCAN_FDT) && + if ((!CONFIG_IS_ENABLED(OF_LIVE) || + (test->flags & UT_TESTF_SCAN_FDT)) && !(test->flags & UT_TESTF_LIVE_TREE) && (CONFIG_IS_ENABLED(OFNODE_MULTI_TREE) || !(test->flags & UT_TESTF_OTHER_FDT)) &&
SPL doesn't have OF_LIVE enabled, so we can only run tests with a flat tree. Don't skip them even if they don't use the devicetree. Fixes: 6ec5178c0ef ("test: Skip flat-tree tests if devicetree is not used") Signed-off-by: Sean Anderson <sean.anderson@seco.com> --- test/test-main.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)