[PATCH weston v5 5/6] Converted the config parser test to the new framework.

[Pekka Paalanen]

On Thu, 02 Jul 2015 18:12:21 -0700
"Jon A. Cruz" <jonc at osg.samsung.com> wrote:

> On 06/25/2015 07:18 AM, Pekka Paalanen wrote:
> > On Sat, 20 Jun 2015 15:47:47 -0700
> > "Jon A. Cruz" <jonc at osg.samsung.com> wrote:
> > 
> >> > +static void *setup_test_config(void *data)
> >> > +{
> >> > +	struct weston_config *config = load_config(data, true);
> >> > +
> >> > +	if (zuc_has_failure())
> >> > +		ZUC_MARK_FATAL("Fixture setup failed.");
> >> >  
> >> >  	return config;
> >> >  }
> >> >  
> >> > -static const char t0[] =
> >> > -	"# nothing in this file...\n";
> >> > +static void *setup_test_config_failing(void *data)
> >> > +{
> >> > +	return load_config(data, false);
> > What if this actually succeeds in loading the config?
> > Do we need a clean-up?
> > 
> 
> As long as the test doesn't crash outright, the object created in the
> setup function will be freed in the tear-down/cleanup function:

Except it does not: in the tests where you expect the loading to fail
the function you quoted below is not hooked up. If the correct result is
getting NULL, then the below would incorrectly flag the test as failed
when it does the right thing.

> >> > +}
> >> > +
> >> > +static void cleanup_test_config(void *data)
> >> > +{
> >> > +	struct weston_config *config = data;
> >> > +	ZUC_ASSERT_TRUE(config != NULL);
> >> > +	weston_config_destroy(config);
> >> > +}
> 
> 
> In general this is the paired approach commonly used for fixtures, and
> should probably get a good high-level writeup. The expectation was that
> as soon as the first patch lands people will actually start asking more
> questions about it and the answers collected up for documentation
> enhancement.

Thanks,
pq