Page MenuHomePhabricator
Create Task
Maniphest T312290

Add some unit and integration test coverage of the PerformTest API in the MW PHP code
Closed, ResolvedPublic
Actions

Description

  • Happy paths
    • Function/implementation/tester as stored persistent objects
    • Function/implementation/tester as inline objects
    • All implementations requested
    • Specific implementations requested
    • All testers requested
    • Specific testers requested
  • Failure paths
    • Running on an unknown ZObject as function
    • Running on a known but non-function ZObject
    • Running on an unknown ZObject as implementation
    • Running on a known but non-implementation ZObject
    • Running on an unknown ZObject as tester
    • Running on a known but non-tester ZObject
    • Implementation raised an error
    • Tester raised an error
    • Response was a failure; expected and actual results are injected
  • Infrastructure failure paths
    • Server not connected
    • Server responded oddly
    • General exception on run

Details

SubjectRepoBranchLines +/-
Throw error when get 404 from orchestrator.mediawiki/extensions/WikiLambdamaster+8 -9
tests: Check that an error occurs in ApiPerformTest when the orchestrator isn't presentmediawiki/extensions/WikiLambdamaster+25 -0
Fix injection of expected and actual values into perform_test result.mediawiki/extensions/WikiLambdamaster+58 -34
Add some more cases to ApiPerformTestTest.php.mediawiki/extensions/WikiLambdamaster+99 -4
ApiPerformTestTest: Annotate some other methods testedmediawiki/extensions/WikiLambdamaster+5 -0
Support receipt of JSON testers in the perform_test API method.mediawiki/extensions/WikiLambdamaster+158 -21
Fix flow where expanded implementation is sent to API for perform_test.mediawiki/extensions/WikiLambdamaster+419 -250
Customize query in gerrit

Event Timeline

Jdforrester-WMF triaged this task as Medium priority.Jul 6 2022, 7:07 PM
Jdforrester-WMF created this task.
Jdforrester-WMF edited projects, added Abstract Wikipedia team (Phase κ – Clean-up); removed Abstract Wikipedia team.Jul 19 2022, 6:44 PM
Jdforrester-WMF moved this task from Incoming to Nice-to-haves on the Abstract Wikipedia team (Phase κ – Clean-up) board.Oct 24 2022, 8:33 PM
EWright claimed this task.Nov 3 2022, 11:30 AM
gerritbot added a comment.Nov 4 2022, 4:35 PM

Change 849120 had a related patch set uploaded (by EWright; author: EWright):

[mediawiki/extensions/WikiLambda@master] Fix flow where expanded implementation is sent to API for perform_test.

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

gerritbot added a project: Patch-For-Review.Nov 4 2022, 4:35 PM
Jdforrester-WMF moved this task from Phase κ – Clean-up to To triage: Nice-to-haves on the Abstract Wikipedia team board.Nov 7 2022, 5:25 PM
Jdforrester-WMF edited projects, added Abstract Wikipedia team; removed Abstract Wikipedia team (Phase κ – Clean-up).
gerritbot added a comment.Nov 7 2022, 5:34 PM

Change 849120 merged by jenkins-bot:

[mediawiki/extensions/WikiLambda@master] Fix flow where expanded implementation is sent to API for perform_test.

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

ReleaseTaggerBot added a project: MW-1.40-notes (1.40.0-wmf.10; 2022-11-14).Nov 7 2022, 6:00 PM
Jdforrester-WMF moved this task to To triage: Un-phased work on the Abstract Wikipedia team board.Nov 7 2022, 6:29 PM
Jdforrester-WMF moved this task from To triage: Un-phased work to To triage: Nice-to-haves on the Abstract Wikipedia team board.
Maintenance_bot removed a project: Patch-For-Review.Nov 7 2022, 6:31 PM
EWright added a comment.EditedNov 8 2022, 10:18 AM

@Jdforrester-WMF I would slightly push back on classing this and other similar testing-related tickets as "nice-to-haves" rather than launch-blocking. If we intend to launch a stable service, I would argue that having at least a bare minimum of test coverage for each important API method is a requirement.

gerritbot added a comment.Nov 11 2022, 2:35 PM

Change 855980 had a related patch set uploaded (by EWright; author: EWright):

[mediawiki/extensions/WikiLambda@master] Support receipt of JSON testers in the perform_test API method.

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

gerritbot added a project: Patch-For-Review.Nov 11 2022, 2:35 PM
gerritbot added a comment.Nov 15 2022, 5:23 PM

Change 855980 merged by jenkins-bot:

[mediawiki/extensions/WikiLambda@master] Support receipt of JSON testers in the perform_test API method.

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

gerritbot added a comment.Nov 15 2022, 5:40 PM

Change 857025 had a related patch set uploaded (by Jforrester; author: Jforrester):

[mediawiki/extensions/WikiLambda@master] ApiPerformTestTest: Annotate some other methods tested

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

gerritbot added a comment.Nov 16 2022, 7:06 PM

Change 857025 merged by jenkins-bot:

[mediawiki/extensions/WikiLambda@master] ApiPerformTestTest: Annotate some other methods tested

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

Jdforrester-WMF updated the task description. (Show Details)Nov 16 2022, 7:55 PM
ReleaseTaggerBot edited projects, added MW-1.40-notes (1.40.0-wmf.12; 2022-11-28); removed MW-1.40-notes (1.40.0-wmf.10; 2022-11-14).Nov 16 2022, 8:00 PM
gerritbot added a comment.Dec 2 2022, 12:54 PM

Change 863287 had a related patch set uploaded (by EWright; author: EWright):

[mediawiki/extensions/WikiLambda@master] Add some more cases to ApiPerformTestTest.php.

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

Jdforrester-WMF updated the task description. (Show Details)Dec 2 2022, 3:36 PM
gerritbot added a comment.Dec 2 2022, 3:48 PM

Change 863287 merged by jenkins-bot:

[mediawiki/extensions/WikiLambda@master] Add some more cases to ApiPerformTestTest.php.

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

EWright added a comment.Dec 2 2022, 3:54 PM

Thanks James for updating the checklist. I think also " Running on an unknown ZObject as tester" is done - see L242: https://gerrit.wikimedia.org/r/plugins/gitiles/mediawiki/extensions/WikiLambda/+/refs/heads/master/tests/phpunit/integration/API/ApiPerformTestTest.php#242. I've updated the checklist.

EWright updated the task description. (Show Details)Dec 2 2022, 3:55 PM

Also, would you mind clarifying what is meant by the item "Response was a failure"? If you mean just that the test fails (returns false), several of the existing test cases cover this.

ReleaseTaggerBot edited projects, added MW-1.40-notes (1.40.0-wmf.13; 2022-12-05); removed MW-1.40-notes (1.40.0-wmf.12; 2022-11-28).Dec 2 2022, 4:00 PM
Jdforrester-WMF added a comment.Dec 2 2022, 4:05 PM
In T312290#8439353, @EWright-WMF wrote:

Thanks James for updating the checklist. I think also " Running on an unknown ZObject as tester" is done - see L242: https://gerrit.wikimedia.org/r/plugins/gitiles/mediawiki/extensions/WikiLambda/+/refs/heads/master/tests/phpunit/integration/API/ApiPerformTestTest.php#242. I've updated the checklist.

Oh, yes, sorry, wrote out the new ones per your update and didn't double-check. Indeed.

In T312290#8439355, @EWright-WMF wrote:

Also, would you mind clarifying what is meant by the item "Response was a failure"? If you mean just that the test fails (returns false), several of the existing test cases cover this.

I think I meant that when the result is 'false' then it also asserts that an expected and actual value were returned.

Jdforrester-WMF updated the task description. (Show Details)Dec 2 2022, 4:05 PM
EWright added a comment.Dec 2 2022, 4:12 PM

Thanks - I'll work on that now.

gerritbot added a comment.Dec 2 2022, 5:20 PM

Change 863377 had a related patch set uploaded (by EWright; author: EWright):

[mediawiki/extensions/WikiLambda@master] Fix injection of expected and actual values into perform_test result.

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

Jdforrester-WMF updated the task description. (Show Details)Dec 2 2022, 7:26 PM
In T312290#8439418, @EWright-WMF wrote:

Thanks - I'll work on that now.

Brill.

gerritbot added a comment.Dec 5 2022, 12:44 PM

Change 863377 merged by EWright:

[mediawiki/extensions/WikiLambda@master] Fix injection of expected and actual values into perform_test result.

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

EWright added a comment.Dec 16 2022, 4:18 PM

Regarding the remaining tasks:

Server not connected
Server responded oddly
General exception on run

It's not obvious to me how to test these. I think we'd need to be able to mock out the Orchestrator, but:

a. Our API integration tests hit an actual MW instance - I don't know how would I set up a test to hit a MW instance that targets a mock Orchestrator
b. I also can't find any examples of simpler unit tests for our API methods that just mock out all the dependencies

If anyone could point me an example of one of the above that I could imitate that would be great.

gerritbot added a comment.Jan 18 2023, 3:51 PM

Change 881446 had a related patch set uploaded (by Jforrester; author: Jforrester):

[mediawiki/extensions/WikiLambda@master] tests: Check that an error occurs in ApiPerformTest when the orchestrator isn't present

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

gerritbot added a comment.Jan 20 2023, 11:04 AM

Change 881446 merged by jenkins-bot:

[mediawiki/extensions/WikiLambda@master] tests: Check that an error occurs in ApiPerformTest when the orchestrator isn't present

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

ReleaseTaggerBot edited projects, added MW-1.40-notes (1.40.0-wmf.20; 2023-01-23); removed MW-1.40-notes (1.40.0-wmf.13; 2022-12-05).Jan 20 2023, 12:00 PM
gerritbot added a comment.Jan 20 2023, 12:33 PM

Change 881867 had a related patch set uploaded (by EWright; author: EWright):

[mediawiki/extensions/WikiLambda@master] Throw error when get 404 from orchestrator.

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

Jdforrester-WMF closed this task as Resolved.Jan 20 2023, 2:54 PM
Jdforrester-WMF updated the task description. (Show Details)

I think we can declare this Resolved. Great work!

gerritbot added a comment.Jan 20 2023, 2:57 PM

Change 881867 merged by jenkins-bot:

[mediawiki/extensions/WikiLambda@master] Throw error when get 404 from orchestrator.

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

Content licensed under Creative Commons Attribution-ShareAlike (CC BY-SA) 4.0 unless otherwise noted; code licensed under GNU General Public License (GPL) 2.0 or later and other open source licenses. By using this site, you agree to the Terms of Use, Privacy Policy, and Code of Conduct. · Wikimedia Foundation · Privacy Policy · Code of Conduct · Terms of Use · Disclaimer · CC-BY-SA · GPL