Page MenuHomePhabricator

Create the first test for api/rest_v1/page/mobile-html/Dog
Closed, DeclinedPublic

Description

Target(s)

Repo

Test

  • open the page
  • check that Dog title is displayed
<h1 data-id="0" class="pcs-edit-section-title">Dog</h1>

Tools

TODO

  • run the test in CI, daily, targeting beta cluster, taking screenshot, sending mail to the group
  • ask @jeena for help with running the test in CI (all it needs to do is to run npm start and npm run selenium-test)

Event Timeline

What's the purpose of this?
Who's the tester in the api team? or a better question, who's the team owner of this api endpoints?

What's the purpose of this?

Both iOS and Android apps just render HTML pages provided by those API endpoints. They had problems in the past where endpoints changed and it broke the apps. Apps teams said the first and easy step could be a few tests for the endpoints.

Who's the tester in the api team?

Nobody, as far as I know.

or a better question, who's the team owner of this api endpoints?

Product Infrastructure, according to https://www.mediawiki.org/wiki/Developers/Maintainers (the repository is https://github.com/wikimedia/mobileapps).

Can we ping someone from that team to check if they have any unit or integration tests?
Make 0 sense to me for us to be creating tests for other teams work

@Jpita @zeljkofilipin This seemed like good low-hanging fruit for first tests to have with the QTE team, since it has recently broken in the past for us and is displayed directly in both iOS and Android apps. Correct me if I'm wrong @JoeWalsh but the thinking was that the existing tests on the mobileapps repo should just be unit tests on the service itself, whereas QTE could write tests against the service at a higher level to test it's integration with other systems. Originally we were thinking against the Beta cluster though there were some concerns that the fact that it's different data could throw things off (Dog already looks different in the Beta cluster vs. Prod). This was the last issue we had, a config change that caused the base domain referenced in the response to be localhost, essentially breaking all the CSS and JS on the screen.

My main concerns with this is (something that happened in both my teams):

Scenario 1

  • we create tests for an endpoint we don't own (running the tests against production)
  • something breaks the tests, by now it's too late because the code is already in production and our users already encounter the bug

Scenario 2

  • we create tests for an endpoint we don't own (running the tests against beta cluster)
  • the tests pass, but then we find a bug in production because the data is different (as you mentioned in the previous comment)

Scenario 3

  • we create tests for an endpoint we don't own (running the tests against beta cluster)
  • assuming the code is deployed to beta cluster when is merged to master BUT before train deployment
  • the tests fail, when we create a ticket and contact the team owner of the endpoint, the code is already merged and the bug is not a priority so the train still runs and the KNOW bug hits production

-- How it should be done --
The team that owns the endpoint:

  • has tests for it running in the CI with every build
  • does not break existing functionalities by creating different versions of the endpoint instead of changing the existing one

Where does the QTE team help:

  • Having one dedicated person on the team OR helping the team with their test plan

@Jpita maybe there's a misunderstanding here. This is supposed to be an experiment, not a test we will maintain forever.

@Jpita maybe there's a misunderstanding here. This is supposed to be an experiment, not a test we will maintain forever.

That’s not mentioned anywhere in the task.
And we’re talking about real problems that happened to our teams so not an experiment, something really needed

The team that owns the endpoint:

has tests for it running in the CI with every build
does not break existing functionalities by creating different versions of the endpoint instead of changing the existing one
Where does the QTE team help:

Having one dedicated person on the team OR helping the team with their test plan

@Jpita in this case, there were two separate services owned by two separate teams involved (RESTBase and the mobileapps service). There are unit tests running with the CI for each service, but as far as I know, there aren't extensive tests of the integration of those two services. The initial thought was to test against the beta cluster since that's the earliest point the two services are integrated. Happy to hear other ideas though - ideally issues would get caught on commit before making it to the beta cluster. CC @WDoranWMF and @dcipoletti if they have other thoughts or ideas. We're specifically trying to prevent more issues like T262437 and T263798 from cropping up as changes are made to the services that currently run through RESTBase.

I think the points/concerns brought up by @Jpita are quite valid. These are some of the experiences that we've had across a number of teams that we've worked with. That being said, there are a lot of separate problems that we need to address that will need to be done in parallel.

When, where, and how something gets tested all have an impact on the usefulness of the test(s). These need to be better defined in an API testing strategy that needs to be developed. This is something that QTE and Platform Infrastructure will partner up to develop.

In the near-term, I think creating some automated tests to cover past known regressions is a useful and interesting first step. Although it doesn't address many of the larger concerns, it does get us going down the path of better understanding the scope of the problem. If it is also something that the Mobile Teams can use as a basic acceptance test of the API, even better.

I've forgotten to reply to this.

As far as I can see, that's just a fixture, a static HTML page that is used by other tests, not the test itself. It does mean that there is a test for it, for example in test/diff/test-spec.js#246

new TestSpec('en.wikipedia.org', 'transform/html/to/mobile-html', ['Dog'], { suffix: 'html', method: 'POST', headers: { 'Content-Type': 'text/html' }, payloadFile: 'test/fixtures/Dog.html' }),

This page is tested, but since it breaks (in context of android/ios apps) from time to time, it's not tested in a way that would catch the problem (for example T262437: [Bug] Page content service is deployed with localhost links to the CSS and JS, breaking all pages that have been edited recently).

Change 633787 had a related patch set uploaded (by Zfilipin; owner: Zfilipin):
[mediawiki/services/mobileapps@master] WIP The first Selenium test

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

Change 633787 abandoned by Zfilipin:
[mediawiki/services/mobileapps@master] WIP The first Selenium test

Reason:

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

zeljkofilipin raised the priority of this task from Medium to Needs Triage.May 17 2021, 1:51 PM