Re: [SSSD] [PATCH] TESTS: Add unit tests for the GPO interface

[Yassir Elley]

----- Original Message -----
> On Tue, Aug 05, 2014 at 11:01:22AM +0200, Jakub Hrozek wrote:
> > On Tue, Jul 22, 2014 at 12:39:24PM -0400, Yassir Elley wrote:
> > > ----- Original Message -----
> > > > On Tue, Jul 22, 2014 at 01:25:50PM +0200, Jakub Hrozek wrote:
> > > > > Hi,
> > > > >
> > > > > I used the attached patch to exercise Yassir's new additions to the
> > > > > sysdb API.
> > > >
> > > > Sorry there was one useless hunk in Makefile.am. A new patch is
> > > > attached.
> > > >
> > > > sssd-devel mailing list
> > > > [e-mail]
> > > > https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
> > >
> > > Looks good to me!
> > >
> > > Thanks,
> > > Yassir.
> >
> > attached is a patch rebased on current master.
>
> rebased again on top of "[PATCH] AD: support gpo processing in offline
> mode"
>
> sssd-devel mailing list
> [e-mail]
> https://lists.fedorahosted.org/mailman/listinfo/sssd-devel

I have one nitpick.

+    ret = sysdb_gpo_store_gpo(test_ctx->domain,
+                              test_guid, 1, 5, 0);
+    fail_if(ret != EOK, "Could not store a test GPO");
+
+    ret = sysdb_gpo_get_gpos(test_ctx, test_ctx->domain, &result);
+    fail_if(ret != EOK, "GPOs not in cache before store op");
+    fail_if(result == NULL);
+    fail_if(result->count != 1);

In the above snippet, you store a gpo and then attempt to retrieve all gpos. The error message after sysdb_gpo_get_gpos says "GPOs not in cache before store op", whereas it should really say "GPOs not in cache after store op".

Otherwise, the code looks good!

Regards,
Yassir.