Page MenuHomePhabricator

Add some additional unit tests
Changes PlannedPublic

Authored by joshuaspence on Nov 15 2015, 11:31 AM.
Tags
None
Referenced Files
Unknown Object (File)
Mon, Jan 27, 9:26 PM
Unknown Object (File)
Fri, Jan 24, 9:58 AM
Unknown Object (File)
Fri, Jan 24, 9:58 AM
Unknown Object (File)
Fri, Jan 24, 9:58 AM
Unknown Object (File)
Wed, Jan 15, 7:38 PM
Unknown Object (File)
Sun, Jan 12, 9:16 PM
Unknown Object (File)
Tue, Jan 7, 1:06 AM
Unknown Object (File)
Fri, Jan 3, 6:15 AM
Subscribers

Details

Reviewers
epriestley
Group Reviewers
Blessed Reviewers
Summary

Add some additional test cases for ArcanistXHPASTLinterRule implementations and alo break down existing test cases into smaller test cases. Depends on D14010.

Test Plan

arc unit

Diff Detail

Repository
rARC Arcanist
Branch
master
Lint
Lint Passed
Unit
Test Failures
Build Status
Buildable 8857
Build 10354: Run Core Tests
Build 10353: arc lint + arc unit

Event Timeline

joshuaspence retitled this revision from to Add some additional unit tests.
joshuaspence updated this object.
joshuaspence edited the test plan for this revision. (Show Details)
joshuaspence added a reviewer: epriestley.
epriestley edited edge metadata.

That build failure looks accurate, see inline.

I'm not really sure that it's valuable to split tests like the define() test apart? I feel like when a test breaks, I often need to look at both positive and negative test cases to fix it. This seems to spread relevant information out without providing an obvious benefit -- it's not like this is breaking 1000-line ultra-tests apart, just splitting tiny tests of the same code (which already produce tailored, relevant errors on failure) apart? That said, I don't have particularly strong feelings either way.

src/lint/linter/xhpast/rules/__tests__/dynamic-define/static.lint-test
4

Test failure on this seems correct?

This revision is now accepted and ready to land.Nov 15 2015, 4:29 PM