Create a script to run test requests for the MediaWiki service
Open, NormalPublic

Description

  1. Create a script to run test requests for the MediaWiki service. This might be checker.py, which @mobrovac recently wrapped into a handy check-<service> tool. If we use checker.py, we'll need to create a very basic swagger spec with x-ample request/response pairs. I'll try to win @mobrovac for this part.

See also: T121597: Implement MediaWiki pre-promote checks

Restricted Application added subscribers: Zppix, Aklapper. · View Herald TranscriptJun 2 2016, 3:33 PM
thcipriani renamed this task from MediaWiki simple canary checks on mw1017 to Create a script to run test requests for the MediaWiki service.Jun 2 2016, 9:04 PM
thcipriani added a project: Deployments.
thcipriani updated the task description. (Show Details)
thcipriani added a subscriber: mobrovac.
Krinkle updated the task description. (Show Details)Jun 2 2016, 10:16 PM
mobrovac triaged this task as Normal priority.
mobrovac added subscribers: Joe, Anomie.

Ok, so to start off here, we need to agree on the way that MediaWiki exposes the test request/response pairs to the checker script. It expects a swagger specification in JSON where each path and method contain an x-amples stanza comprised of request and response objects. It sends the request object information and checks the return value against the one provided in the spec.

One way I see that might be easy to implement is to expose the swagger spec at /w/api.php?spec. AFAIK, that should be easy to add to api.php while at the same time it would keep the compatibility with the checker script. What do you guys think?

A separate problem for which we also need to find a solution is how to fill the spec. Should it be done manually so as to check only for the most important features? Or perhaps allow extensions and API modules to add their own paths/methods?

One way I see that might be easy to implement is to expose the swagger spec at /w/api.php?spec.

That does not fit the existing structure of api.php. More likely would be something along the lines of /w/api.php?action=swaggerspec.

Beyond that, this task is lacking in basic details such as "what is a swagger spec?" and "what does this mystery script do with it?" if you're seeking my input.

Anomie added a comment.Jul 8 2016, 3:30 PM

Thanks, that was useful.

Some notes:

  • The paths object mechanism doesn't seem like it would be able to really handle the action API, considering every request has at least two independent parts (the action and the format) and might have many more (e.g. query with its many submodules, or several other actions with generators).
    • We'd probably wind up having only a single "/api.php" path that had to somehow handle everything. In which case there's no hope for it to really document the API since different actions are free to use the same parameter name for completely different purposes.
    • Of course, we don't necessarily have to care if the goal here is just to dump some data for this script instead of producing a generally useful OpenAPI spec. In that case we probably shouldn't name it as if it were generating anything like a generally useful OpenAPI spec; maybe "action=x-amples" would be the most accurate name then.
  • The spec has "query" parameters and "formData" parameters, but not parameters that could validly be either one. Unless maybe you define each of them twice, once with each location.
  • It's not clear whether parameters declared as "type: boolean" mean they're like HTML checkboxes or whether they're supposed to have values of 'true'/'false'. It generally punts over to JSON schema documentation for data types, but that doesn't seem like it translates very well to non-JSON mime types.
  • The responses are similarly hopeless, both because of the "multiple independent parts" issue and because the API doesn't normally consider API-level errors to be transport-level errors (T40716).

If we go with the "don't worry about making anything like a useful OpenAPI spec, just do the minimum required by the test script" plan, it should be doable without a huge amount of trouble. What I'd recommend:

  • A new method on ApiBase for subclasses to override, probably ApiBase::getTestData(), would return a data structure appropriate for ApiXAmples to be able to generate one or more request/response pairs as described at https://github.com/earldouglas/swagger-test#direct-test-specification.
    • This might be able to partially replace ApiBase::getExamplesMessages(), since the 'description' and 'params' fields would be basically the same information that getExamplesMessages returns. I say "partially" because of the potential difficulty mentioned below, someone could still provide useful examples even if they can't make functional x-amples.
    • I'd recommend against using exactly that format, though; instead, probably [ 'description' => /* something accepted by ApiBase::makeMessage() */, 'params' => [ ... ], 'status' => 200, 'headers' => [...], 'result' => [ ... ] ]. "status" and "headers" would be optional because they'll almost always be 200 and [], respectively.
    • I note you might run into difficulty when extensions can modify the output of a module, or when a module's output depends too heavily on the content of the wiki.
  • ApiXAmples would operate much like action=help&recursivesubmodules=1 by default, iterating the module tree recursively and combining all the x-amples, but could also have modules, submodules, and recursivesubmodules parameters that work like ApiHelp to allow for testing a single module.
  • ApiParamInfo might want to output the x-amples too, although without the reformatting into x-amples format.
  • Something to run these tests from Jenkins, perhaps a phpunit 'structure' test.

In case it was useful, @Legoktm and I had T100903: Run pywikibot test suite regularly on beta cluster as part of MediaWiki/Wikimedia CI working briefly. It shouldnt take much effort to get it operational again, and to run the same suite against a pre-promote instance.

@Anomie thnx a lot for the comments! You are way ahead of what I had in mind for the first version. For the first iteration, I think it's ok to have a static YAML spec that is edited manually. Something like:

mw-spec.yaml
swagger: '2.0'
info:
  version: 1.28.0
  title: MediaWiki
  description: THE Wiki software solution
  termsOfService: https://wikimediafoundation.org/wiki/Terms_of_Use
paths:
  /wiki/{title}:
    get:
      x-amples:
        - title: en.wp Main Page
          request:
            params:
              title: Main_Page
            headers:
              host: en.wikipedia.org
          reponse:
            status: 200
            headers:
              content-type: /text\/html/
            body: /Wikipedia, the free encyclopedia/
        - title: en.wp Special Version
          request:
            params:
              title: Special:Version
            headers:
              host: en.wikipedia.org
          reponse:
            status: 200
            headers:
              content-type: /text\/html/
            body: /<title>Version - Wikipedia/
  /w/api.php:
    post:
      x-amples:
        - title: en.wp Main Page pageprops
          request:
            query:
              action: query
              prop: pageprops
              titles: Main_Page
              format: json
              formatversion: 2
            headers:
              host: en.wikipedia.org
              accept: application/json
          response:
            status: 200
            headers:
              content-type: /application\/json/
            body: /.+/

This would at least test the appservers for some basic functionality on a regular basis and during deploys.

In that case, you don't really need api.php to return it at all. Just put your file in /w/ and point to it.

mobrovac claimed this task.Aug 25 2016, 5:49 PM

Change 306707 had a related patch set uploaded (by Mobrovac):
Allow service-checker to read YAML-formatted specs

https://gerrit.wikimedia.org/r/306707

Change 306709 had a related patch set uploaded (by Mobrovac):
Expose a simple Swagger spec for checks

https://gerrit.wikimedia.org/r/306709

Change 307913 had a related patch set uploaded (by Legoktm):
[WIP] API: Add action=x-amples to autogenerate a swagger spec

https://gerrit.wikimedia.org/r/307913

I wrote a rather simple autogenerated API spec based on Anomie's advice above. It requires the service-checker patch https://gerrit.wikimedia.org/r/#/c/307910/ to work properly if your script path is not the root.

$ service-checker-swagger 127.0.0.1 http://localhost/ -s "/vagrant/api.php?action=x-amples&format=json&formatversion=2"
All endpoints are healthy

It just checks that the API responds with a response code of 200 (which it does for basically every request, including errors), so it's not that useful yet, but that will require writing specs for individual modules which should be easy once there's a framework.

Change 306707 merged by jenkins-bot:
Allow service-checker to read YAML-formatted specs

https://gerrit.wikimedia.org/r/306707

It seems like all that's missing to incorporate this into deployments and monitoring is:

  1. Some way to fetch the swagger spec from an app/api servers
  2. A rebuild of the python-service-checker package to include the new yaml patch (wasn't there when I installed on deployment-tin today)
  3. Install of python-service-checker on the deployment host

To point 1: I'm all in favor of perfect not being the enemy of good enough and in this instance it seems like that's just throwing a spec.yaml file under /w/ in mediawiki-config should work (unless there are strenuous objections to that and/or the action=x-amples work is close to completion).

I ran a basic test of this today in beta. I installed python-service-checker and manually applied the patch in https://gerrit.wikimedia.org/r/306707. Then I put the proposed simple spec (from https://gerrit.wikimedia.org/r/306709 (sans a few headers)) in /w/ on deployment-mediawiki05

From deployment-tin I was able to run:

thcipriani@deployment-tin:~$ service-checker-swagger deployment-mediawiki05.deployment-prep.eqiad.wmflabs http://deployment.wikimedia.beta.wmflabs.org -s "/w/spec.yaml"
All endpoints are healthy

These initial rudimentary checks would probably add some peace-of-mind for deployments, which is why I'm pinging this task to check status :)