Page MenuHomePhabricator

Add better context to the output of e2e tests that fail
Closed, ResolvedPublic

Description

Problem

TL;DR: the error message when an e2e test fails usually doesn't contain enough information to diagnose the issue.

The general format of the e2e tests is: setup the environment, make a REST API request, assert several key parts of the response object are as expected.

const response = await newGetItemStatementsRequestBuilder( testItemId )
    .assertValidRequest()
    .makeRequest();

assert.equal( response.status, 200 );
assert.equal( response.header[ 'last-modified' ], testModified );
assert.equal( response.header.etag, makeEtag( testRevisionId ) );
// more assert statements testing response body etc...

As the response is usually verified to be correct via several different assert statements, if the response is different to expected you'll only ever see the failure message of the first assert statement that failed. This is fine if, in the example above, the issue is that the last-modified header is incorrect, as you probably have enough context in the error message (expected value and actual value). However, if the response was a 400 error response, then the first statement asserting that the status code is 200 will fail and only provide you with the the expected value (200) and the actual value (400) as the context. Being able to provide more context, such as the body to see what the error is, would be useful.

Solutions considered rejected

  1. Add/remove console.log statement(s) each time you need to debug an issue
    • Pros
      • Doesn't require any custom code to be written and maintained
    • Cons
      • The console output would look out of place injected into the normal, well formatted, test output
      • The console output is displayed when run and not after all the tests have run with the normal test summary
      • We don't allow console calls in our JS tests (869186) so would have to remove them before merging
      • Annoying / time consuming to have to add and then remove console statements
  2. Add console.log statement(s) before assert statements
    • Pros
      • Doesn't require any custom code to be written and maintained
    • Cons
      • It would massively clutter up the test output making it difficult to find the needed context
      • The console output would look out of place injected into the normal, well formatted, test output
      • The console output is displayed when run and not after all the tests have run with the normal test summary
      • We don't allow console calls in our JS tests (869186)
  3. Add some context to the assert statements
    • Pros
      • Doesn't require any custom code to be written and maintained
      • Is only displayed if there is an error
      • Is displayed after all the tests have run in the summary section
    • Cons
      • Lots of work, have to do it for each assert statement in each test
      • Would have to add it as the assert statement's message parameter
        • A bit of a hack / not what the message param is designed for
        • The message param is a string so isn't great at displaying objects or multiple information. Would probably require converting to JSON.
        • We use the message param in some of our tests for its intended purpose
      • Still displays poorly along side the normal test output

Event Timeline

Change 841114 had a related patch set uploaded (by Ollie Shotton; author: Ollie Shotton):

[mediawiki/extensions/Wikibase@master] REST: Improve the output of e2e tests that fail

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

Ollie.Shotton_WMDE renamed this task from Improve the output of e2e tests that fail to Add better context to the output of e2e tests that fail.Dec 22 2022, 2:30 PM

Change 908796 had a related patch set uploaded (by Ollie Shotton; author: Ollie Shotton):

[mediawiki/extensions/Wikibase@master] REST: Custom chai method to assert response status

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

Change 908796 merged by jenkins-bot:

[mediawiki/extensions/Wikibase@master] REST: Custom chai method to assert response status

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

Change 841114 abandoned by Ollie Shotton:

[mediawiki/extensions/Wikibase@master] REST: Improve the output of e2e tests that fail

Reason:

in favour of I9ffff7e330edf8091b305af4fe9f6c94d4d749c9

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