On Mon, Oct 21, 2019 at 02:08:23PM +0300, Jarkko Sakkinen wrote: > On Thu, Oct 17, 2019 at 11:13:09AM -0700, Sean Christopherson wrote: > > On Thu, Oct 17, 2019 at 07:53:56PM +0300, Jarkko Sakkinen wrote: > > > On Wed, Oct 16, 2019 at 08:03:34PM -0700, Sean Christopherson wrote: > > > > Move the operator macros, ASSERT_* and EXTEND_*, to a standalone header > > > > so that they can be reused by other selftests without pulling in the > > > > full harness framework, which is cumbersome to use for testing features > > > > that require a substantial amount of setup, need callbacks, etc... > > > > > > > > Signed-off-by: Sean Christopherson <sean.j.christopherson@xxxxxxxxx> > > > > > > Is it possible to just use a "dull" selftest and not go into this before > > > the code is upstreamed? If yes, lets go with that. > > > > It's certainly possible, but the code is verbose and ugly (IMO), which > > means it will be harder for other to review. > > Ok, I'll try to explain in more verbose terms how I see this. > > Not all selftests use the harness and I'm not yet confident that SGX has > to. Unfortunately, ugly is for me something that I cannot put metrics > on. Also, often "ugly" is actually better than layering because it is > more transparent. > > The test is comprised of simple POSIX calls that everyone knows whereas > using kselftest harness requires learning new framework. Less macros > makes code also easier to debug and pair compare to dissembly when > required. I've done the latter at least a few times. > > It will also add a requirement for code reviewers who are simply looking > for a code example how SGX works also to learn the harness. In the scope > of the patch set the selftest serves as a such example. Eh, if SGX were actually using any of the harness stuff, sure, but I'd hope most reviewers intuitively understand what ASSERT_EQ does.
