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)
Tue, Dec 10, 9:32 AM
Unknown Object (File)
Thu, Dec 5, 7:55 PM
Unknown Object (File)
Wed, Nov 27, 9:24 AM
Unknown Object (File)
Nov 24 2024, 11:52 AM
Unknown Object (File)
Nov 22 2024, 2:36 AM
Unknown Object (File)
Nov 11 2024, 3:45 AM
Unknown Object (File)
Nov 8 2024, 10:46 PM
Unknown Object (File)
Oct 31 2024, 12:12 PM
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