Most of Phabricator is untestable with unit tests. Browser testing is the way to ensure that functionality is not broken with upstream pulls.
Description
Description
Details
Details
Related Changes in Gerrit:
| Subject | Repo | Branch | Lines +/- | |
|---|---|---|---|---|
| adds base phabricator.sprint.selenium browser test package | phabricator/extensions/Sprint | master | +943 -0 |
| Status | Subtype | Assigned | Task | ||
|---|---|---|---|---|---|
| Resolved | Aklapper | T94265 Goal: Consolidate Phabricator as project management tool | |||
| Declined | None | T85123 Create a continuous integration plan for Wikimedia Phabricator patches | |||
| Resolved | dduvall | T87359 Create Browser Tests for Phabricator |
Event Timeline
Comment Actions
This isn't really true - phabricator has it's own unit testing engine and there is a decent amount of test coverage. Run 'arc unit' within the phabricator checkout to see what I mean ...
Comment Actions
Additionally, a lot of phabricator's internal objects have a makeEphemeral method that prevents the object from getting committed to storage - this is useful when you need to create objects within a unit test and you would like to avoid polluting storage with a bunch of test data.
Comment Actions
We have been kind of pessimistic about the actual outcome of doing this but maybe we could at least get a reasonable way down this road using the native arc tests.
Comment Actions
well, unfortunately the unit testable objects have little relationship to the projects or maniphest controller logic, which is what needs to be tested for the Sprint extension at least. The processRequest() function typically is not encapsulated and has a lot of breakable stuff. Very few of the controllers are tested as far as I can tell. I have bootstrapped the phabricator phpunit engine and wrote some tests, but the coverage is very limited (43%).
See this file for a report of the unit tested controllers
. Because of this, I really think that browser testing is reasonable at any rate.Comment Actions
Change 188549 had a related patch set uploaded (by Christopher Johnson (WMDE)):
adds base phabricator.sprint.selenium browser test package
Comment Actions
Change 188549 merged by Christopher Johnson (WMDE):
adds base phabricator.sprint.selenium browser test package
Comment Actions
Was this supposed to be just for the sprint extension? If so, that seems done.
Otherwise, we need some more specificity on what's next/needed (sub-tasks?).
Comment Actions
The browser tests in the Sprint extension are very limited and cover only a sampling of stuff. The base selenium junit framework that is in Sprint could easily be expanded and developed for integration testing at a higher level though. I am not sure what is going on currently with the overall CI question with the transition from Gerrit to Differential T18, but I think that this transition plan would be first in the sequence, and more sophisticated browser testing would follow. This does not mean that the browser tests still cannot be run manually, which may be an option in the interim.
Comment Actions
Removed release-engineering, looks like this is not a task for that board.
Removed patch-for-review, the only patch was merged.
Since the only patch is merged, is there anything else that needs to be done here, or can the task be closed?
Comment Actions
FWICT, the commit associated with this task was completed. If you want to do more with browser tests against Phabricator, open a new task and let us know if you want help or to pair on it. Thanks!