Page MenuHomePhabricator

Add iOS test to see if beta cluster introduces unbalanced div or span to mobileview section html output
Closed, ResolvedPublic

Description

Hopefully this test allows us to take early action for issues like T165115.

I added a new test which...

  • requests mobileview output from beta cluster for a specific revision of an article on the beta cluster
  • ensures equal number of opening and closing divs. same for spans.

That way we'll get notified the same day any unbalanced divs or spans land in beta mobileview section html. (so we can catch it before it lands in prod!)

Event Timeline

Mhurd created this task.May 12 2017, 11:38 PM
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptMay 12 2017, 11:38 PM
Mhurd renamed this task from Add test to see if beta cluster changes anything about mobileview section html output to Add iOS test to see if beta cluster changes anything about mobileview section html output .May 12 2017, 11:38 PM
Mhurd claimed this task.EditedMay 13 2017, 1:17 AM
Mhurd added subscribers: Niedzielski, Mholloway, Dbrant.

@Niedzielski @Mholloway @Dbrant

Adding you folks in case a quick port of this test would be useful as and early warning of staged changes affecting section html for Android too. (and maybe a MCS version for un-ignorable heads-up/reminder for changes to its output too?)

Mhurd added a comment.EditedMay 13 2017, 1:28 AM

Also let me know if this approach sounds like it won't accomplish what I'm trying to achieve! :-P

I think @bearND does this for the Content Service so maybe he can share pros and cons of this approach. My opinion is that this kind of test is the highest level possible with the following pros and cons:

  • +/- All changes are breaking changes; we always know when something changes but our tests break (and must be repaired) constantly.
  • + A clear record is kept for all concrete changes on a given page. The buck stops here!
  • - Identifying whether a change has negative effects requires manual investigation.

In my opinion, if we're able to identify breaking scenarios, we should write tests for them upstream. I would rather set a cron job to just version the output of select pages so we could see a history of how they changed and maybe flag those changes on an IRC channel but I'm not super interested in maintaining a test.

bearND added a comment.EditedMay 15 2017, 5:31 PM

Yes, I do this for MCS, where the content comes from Parsoid. You can check in the MCS repo the test/diff folder. Initially I started out with specific revision but it turns out that this was not enough. I also added specific time uuids (a RESTBase feature) to get the tests more stable. The issue is that template inclusions and even pageimages can change things subtly even when requesting the same revision.
Initially I was also using regular pages. But now I prefer a couple of custom made test pages in User space (mainly a page I collected body parts from other pages from) which tend to only get edited by myself and where I curate which templates are used.
I don't have this running regularly yet. The test just runs as part of the regular CI, when someone pushes a patch update. When there are diffs I look at them from my IDE, which has a nice diff viewer. I have two levels, the whole JSON file, and a second level where every section text is just its own HTML file. I think the JSON level is good enough for most changes.

Thanks for the helpful detail @bearND!

JMinor triaged this task as Medium priority.May 15 2017, 6:34 PM
Mhurd added a comment.May 15 2017, 7:09 PM

@bearND

But now I prefer a couple of custom made test pages in User space (mainly a page I collected body parts from other pages from) which tend to only get edited by myself and where I curate which templates are used.

My hope was that having this test go against a specific historical revision would make the test immune to editing changes... of course the beta instance could blast the article history at some point, but thought it was worth a try at least :)

Mhurd added a comment.May 15 2017, 7:22 PM

@bearND

Oops I missed the first part of your comment somehow :-P

Yes, I do this for MCS, where the content comes from Parsoid. You can check in the MCS repo the test/diff folder. Initially I started out with specific revision but it turns out that this was not enough.

Do you think my test would be better if I created a simpler page in my user space on beta then tested against revision of it?

bearND added a comment.EditedMay 16 2017, 4:25 PM

@Mhurd Well, that's what I ended up with. In user space you have more control. I also recommend testing against a specific revision. You may want to check out my test pages and the Node.js code for running the tests.

Mhurd added a subscriber: JoeWalsh.EditedMay 18 2017, 7:05 PM

Note to self: Per @JoeWalsh make this test actually check for unbalanced div's so we don't have failures on every string change.

JMinor lowered the priority of this task from Medium to Low.May 23 2017, 6:22 PM
Mhurd updated the task description. (Show Details)EditedMay 26 2017, 12:13 AM

I updated the ticket description to reflect the comment above. New PR here https://github.com/wikimedia/wikipedia-ios/pull/1465

Mhurd renamed this task from Add iOS test to see if beta cluster changes anything about mobileview section html output to Add iOS test to see if beta cluster introduces unbalanced div or span to mobileview section html output .May 26 2017, 12:15 AM
Mhurd updated the task description. (Show Details)
JMinor closed this task as Resolved.Jun 12 2017, 7:21 PM