On Thu, 31 Aug 2017 14:37:45 +0200
ludo@gnu.org (Ludovic Courtès) wrote:

> Christopher Baines <mail@cbaines.net> skribis:
> 
> > * gnu/tests/databases.scm (%test-mongodb): New variable.  
> 
> FWIW I’d suggest squashing it with the patch that adds the MongoDB
> service, since they really go together.

Ok, I've merged these patches together now.
 
> > +(define %mongodb-os
> > +  (operating-system
> > +    (inherit
> > +     (simple-operating-system
> > +      (dhcp-client-service)
> > +      (service mongodb-service-type)
> > +      (extra-special-file "/etc/os-release"
> > +                          (plain-file "os-release" "guix\n"))))  
> 
> Does it require that ‘os-release’ file?  If so, should the service add
> it?  That would probably intrusive though, so it’s even better if
> MongoDB does not require it.

I think this was me trying to get the tests to pass, but it didn't
help, as the mongo client was running outside of the test system. I've
now removed it.

> > +          (test-eq "can connect"
> > +            0
> > +            (system* (string-append #$mongodb "/bin/mongo")
> > +                     "test"
> > +                     "--eval"
> > +                     "help"))  
> 
> If it’s easy to do through the CLI, it might be worth trying insert an
> element and query it.

I've done this now. Also, I've included a patch from upstream that
fixes the crash in the client that was causing the system test to fail,
so the test should now pass.

> > +%mongodb-os  
> 
> Leftover.

I've removed this now.

> Otherwise LGTM, thank you!

Great, thanks for reviewing Ludo :)