Re: [OE-core] Rust Oe-Selftest implementation V15 Testing

[Richard Purdie <richard.purdie@linuxfoundation.org>]

On Wed, 2023-07-19 at 12:58 +0100, Alex Kiernan wrote:
> On Wed, Jul 19, 2023 at 11:49 AM Richard Purdie
> <richard.purdie@linuxfoundation.org> wrote:
> > 
> > On Mon, 2023-07-17 at 16:08 +0100, Richard Purdie wrote:
> > > On Mon, 2023-07-17 at 16:34 +0200, Alexandre Belloni wrote:
> > > > Hello,
> > > > 
> > > > I got some feedback from RP:
> > > > 
> > > > http://autobuilder.yocto.io/pub/non-release/20230716-18/testresults/testresult-report.txt
> > > > 
> > > > This shows warnings for duplicate tests and he also asks being where the
> > > > qemuarm64 went, both issues being probably related.
> > > 
> > > I had a look and you probably need to add a:
> > > 
> > > @OETestTag("toolchain-user")
> > > @OETestTag("runqemu")
> > > class RustSelfTestBase(RustSelfTestSystemEmulated):
> > >     def test_check(self):
> > >         self.test_rust()
> > > 
> > > section to the tests to allow the tests to run for the non-IA
> > > architectures that use usermode emulation for the other tests which
> > > rust can't.
> > > 
> > > That then just brings the question of why there are duplicate tests
> > > results being reported. This is where the result for an ID is being
> > > reported more than once. I haven't looked into why that might be
> > > happening.
> > 
> > I really do want to get this rust test suite issue resolved so I went
> > digging into the code to find out what is really going on.
> > 
> > Firstly, the duplicate test results. The issue is that you defined the
> > core class like this:
> > 
> >  class RustSelfTestSystemEmulated(OESelftestTestCase, OEPTestResultTestCase):
> >      def test_rust
> > 
> > and python unittest has a convention where anything starting "test_" is
> > a test.
> > 
> > This meant that the rust test ran unguarded in all the oe-selftest
> > targets on the autobuilder and not just in the toolchain-system
> > filtered section.
> > 
> > The easiest fix is to drop the RustSelfTestBase class and move the
> > toolchain-system decorator to RustSelfTestSystemEmulated. That will
> > resolve the duplicate test warnings and ensure things run where they
> > should. You could have worked out this issue by finding that there were
> > rust test results in oe-selftest-* testresults.json files, e.g. here:
> > 
> > http://autobuilder.yocto.io/pub/non-release/20230716-18/testresults/oe-selftest-centos/oeqa/testresults.json
> > 
> > Moving on, the test result names really don't look good with the
> > "[ui]  " and similar prefixes in the results file. I've patched a tweak
> > in to drop that.
> > 
> > I also noticed that there were no skipped tests being reported in the
> > results. This was due to "SKIP" being used instead of "SKIPPED" which
> > resulttool looks for.
> > 
> > We need to also add the toolchain-user decorator to make sure that the
> > tests run for the "user" architectures since we don't have any user
> > mode rust test equivalent.
> > 
> > I've rolled all these changes into a patch on master-next:
> > 
> > https://git.yoctoproject.org/poky/commit/?h=master-next&id=46ab84785da15ac156ee0b4a693ce8bb5ccf8c22
> > 
> > which I'll put into testing.
> > 
> > Looking to the future, I have concerns about the ease of maintenance of
> > this huge patch to rust to disable failing tests. I'd propose we change
> > this to a hardcoded list of tests to ignore in the result parsing code
> > which will be easier to maintain in the future.
> > 
> 
> That feels far more maintainable - the scale of the  patch really
> concerned me. Also I guess if it's just a list, then running with
> everything enabled on upgrade to see what passes/fails would be a
> relatively easy option.

I've gone ahead and merged the rust selftest changes since we had a
successful test run on the autobuilder and I think that patch series
has gone on long enough. I pushed my tweaks in a follow up commit.

I'd happily take a patch changing the current patch for the approach
mentioned above as a next step. I appreciate it will mean we need to
tweak the 1.71.0 update but felt getting the tests in was probably the
next logical step.

Cheers,

Richard