Testing endpoint services in puppet-server

[Brice Figureau]

Hi all,

I've started coding the file_metadata terminus as a pure clojure service
in the puppet-server and have a few existencial questions.

I have a basic fileserver.conf parsing system, mount finding from a
path, and a compojure app (and trapperkeeper service) for file_metadata
(not yet file_metadatas support).

First question, the code uses the jdk 1.7 java.nio.file system to access
posix/unix file properties like uid or permissions. That means the
server must run on an 1.7 jvm. Is that an issue?

The second problem I'm facing is testing the service. I have a coverage
of the various core functions (the ones producing the metadata
themself), but I struggle to see how to test whole endpoint from a given
request.

Is there any pointers I should look at?

And finally, would it be OK for me to publish the current code as a PR
(not to be merged) to gather comments and reviews of the code, as this
is my first clojure code, there's certainly tons of issues :)

Thanks,
--
Brice Figureau
My Blog: http://www.masterzen.fr/

[Chris Price]

On Sat, Dec 27, 2014 at 8:39 AM, Brice Figureau <brice-...@daysofwonder.com> wrote:

Hi all,

I've started coding the file_metadata terminus as a pure clojure service
in the puppet-server and have a few existencial questions.

I have a basic fileserver.conf parsing system, mount finding from a
path, and a compojure app (and trapperkeeper service) for file_metadata
(not yet file_metadatas support).

Nice!

 

First question, the code uses the jdk 1.7 java.nio.file system to access
posix/unix file properties like uid or permissions. That means the
server must run on an 1.7 jvm. Is that an issue?

Not at all; Jetty 9 already requires Java 7, so, no worries there.

The second problem I'm facing is testing the service. I have a coverage
of the various core functions (the ones producing the metadata
themself), but I struggle to see how to test whole endpoint from a given
request.

Is there any pointers I should look at?

There are several different levels that you can test at, and it can be a bit tricky to determine the most appropriate one... here are a few examples.

If you want to test the ring request/response logic but you don't really need to spin up a full Jetty instance to do it, you can do something like this:

https://github.com/puppetlabs/puppet-server/blob/dc81dbca89b206e00163c7d53d806e6c51c33ed0/test/unit/puppetlabs/services/master/master_core_test.clj#L12-L13

If you want to spin up a Jetty to test a ring app but you don't need the full trapperkeeper stack, we have the with-test-webserver utility in the tk-j9 testutils:

Docs: https://github.com/puppetlabs/trapperkeeper-webserver-jetty9/blob/d1f4fea3be52a937bd2f82fc512d274022cc8f49/doc/test-utils.md#with-test-webserver

Example usage: https://github.com/puppetlabs/trapperkeeper-webserver-jetty9/blob/ad61d23dd7d3ccd7ebf96d93bc0b684d058d335d/test/clj/puppetlabs/trapperkeeper/services/webserver/jetty9_core_test.clj#L67

If you want to test via a trapperkeeper instance but w/o the full Puppet Server stack, you can use the TK testutils, e.g. `with-app-with-config`:

Docs: https://github.com/puppetlabs/trapperkeeper/wiki/Test-Utils#with-app-with-config

Example: https://github.com/puppetlabs/puppet-server/blob/0b5118f89d78e17e473fbdecd5414a7307b3668c/test/unit/puppetlabs/services/jruby/jruby_puppet_service_test.clj#L31

Finally, if you need a full death-star Puppet Server integration test (which we don't find we usually need), we have a new testutil macro called `with-puppetserver-running`:

https://github.com/puppetlabs/puppet-server/blob/dc81dbca89b206e00163c7d53d806e6c51c33ed0/test/integration/puppetlabs/services/jruby/puppet_environments_int_test.clj#L112

That last one is pretty new and we probably need to build up some more helper functions, test utilities, and best practices around it, though.

And finally, would it be OK for me to publish the current code as a PR
(not to be merged) to gather comments and reviews of the code, as this
is my first clojure code, there's certainly tons of issues :)

Definitely!  If you could just prefix the subject with "(FOR REVIEW ONLY)" or similar, that would be helpful... and we will have to prioritize review against other deliverables, obviously, but we'd be happy to look at it.

On a related note: we need to either start including Puppet Server tickets/PRs in the weekly public OSS PR triage meetings, or we need to start scheduling some public triage meetings specifically for Puppet Server.  Any thoughts?

[Brice Figureau]

Hi Chris,

Thanks for the answer, I'm in the process of pushing the for-review-only PR.

See below for some additional comments.

On 28/12/2014 14:13, Chris Price wrote:
> On Sat, Dec 27, 2014 at 8:39 AM, Brice Figureau

> <brice-...@daysofwonder.com <mailto:brice-...@daysofwonder.com>>

I started writing the test before reading your answer and went with this
last solution. It has the merit of testing the full stack.

I will see how I can add some lighter tests. My problem was testing that
my routes and ring-handler were behaving correctly. I'll study the
pointers you sent to see if there's something lighter than the full
stack that can apply.

> And finally, would it be OK for me to publish the current code as a PR
> (not to be merged) to gather comments and reviews of the code, as this
> is my first clojure code, there's certainly tons of issues :)
>
>
> Definitely! If you could just prefix the subject with "(FOR REVIEW
> ONLY)" or similar, that would be helpful... and we will have to
> prioritize review against other deliverables, obviously, but we'd be
> happy to look at it.

Of course, I don't expect an immediate answer :)
Based on my spare time this days, I think I will slowly try to improve
the PR.

> On a related note: we need to either start including Puppet Server
> tickets/PRs in the weekly public OSS PR triage meetings, or we need to
> start scheduling some public triage meetings specifically for Puppet
> Server. Any thoughts?

It all depends on how much external PR you expect. I think it might make
sense at start to just include those PRs in the weekly triage meetings.
Then if the number of PRs increases, it might be time to schedule a
specific triage meetings.

[Brice Figureau]

On 28/12/2014 14:13, Chris Price wrote:

> On Sat, Dec 27, 2014 at 8:39 AM, Brice Figureau

> <brice-...@daysofwonder.com <mailto:brice-...@daysofwonder.com>>

So I tried two of the above methods with great success, but in the case
I'm focusing on it's not enough.

Since it's a reimplementation of one of the ruby puppet service, I think
what I should test is that the outcome of a given request by the clojure
code is equivalent to what the ruby part would have returned.
I think this is doable by using calling the request-handler service and
comparing the result to what the clojure service would return.

What do you think about that?

[Kevin Corcoran]

Since it's a reimplementation of one of the ruby puppet service, I think
what I should test is that the outcome of a given request by the clojure
code is equivalent to what the ruby part would have returned.
I think this is doable by using calling the request-handler service and
comparing the result to what the clojure service would return.

Hi, Brice.

That's an interesting idea.  I think that would be a very valuable manual test to run initially.  However, long-term, I expect we will be deleting that functionality from the ruby codebase, at which point this test would have to be deleted (or re-written) as well.

Do the existing acceptance tests in the 'puppet' repo cover this functionality?  If so, that might be sufficient coverage at this 'level' of testing.  You're not adding new functionality (from a user's perspective) ... so maybe the test coverage you're hoping to get is already written in the acceptance tests.

We run the entire acceptance test suite in the puppet repo against puppet server in CI: https://jenkins.puppetlabs.com/view/Puppet%20Server/view/All/job/platform_puppet-server_integration-system_full-master/