Test Plan: Ran PhpunitTestEngine unit test. Also used refactored PhpunitTestEngine to run phpunit tests.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin, aurelijus
Differential Revision: https://secure.phabricator.com/D4651
Summary: Adds "arc unit --everything", which runs every available test, provided the test engine supports it. Also add JSON output.
Test Plan: Ran `arc unit --everything` in arcanist/, libphutil/ and phabricator/. Saw all tests run.
Reviewers: btrahan, vrana
Reviewed By: vrana
CC: aran
Maniphest Tasks: T2065
Differential Revision: https://secure.phabricator.com/D4214
Summary:
This commit doesn't change license of any file. It just makes the license implicit (inherited from LICENSE file in the root directory).
We are removing the headers for these reasons:
- It wastes space in editors, less code is visible in editor upon opening a file.
- It brings noise to diff of the first change of any file every year.
- It confuses Git file copy detection when creating small files.
- We don't have an explicit license header in other files (JS, CSS, images, documentation).
- Using license header in every file is not obligatory: http://www.apache.org/dev/apply-license.html#new.
This change is approved by Alma Chao (Lead Open Source and IP Counsel at Facebook).
Test Plan: Verified that the license survived only in unit tests and LICENSE file.
Reviewers: epriestley, btrahan, edward
Reviewed By: epriestley
CC: aran, Korvin, davidrecordon
Maniphest Tasks: T2035
Differential Revision: https://secure.phabricator.com/D3881
Summary: Currently, a shutdown exception ("script exited with open transactions!") overwhelms the actual test failure exception, which is the one that needs to be fixed.
Test Plan: Ran a fixture test which opens a transaction and then throws. Got diagnostically useful output after this patch.
Reviewers: vrana, btrahan
Reviewed By: vrana
CC: aran
Differential Revision: https://secure.phabricator.com/D3780
Summary:
See https://github.com/facebook/arcanist/pull/53
- Work correctly for directories with `%` in their name; this breaks under sprintf().
- Search for `src/` -> `tests/` style directories.
- Add coverage for search paths.
Test Plan:
Ran unit tests.
I don't have a working test case for PHPUnit tests, can one of you guys apply this and verify I didn't break your setups?
Reviewers: quard, aurelijus
Reviewed By: aurelijus
CC: aran
Differential Revision: https://secure.phabricator.com/D3558
Summary: I want to use it from outside.
Test Plan:
$ arc unit src/lint/linter/__tests__/
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D3493
Summary: This is Arcanist part of D3434.
Test Plan: None.
Reviewers: epriestley, wez
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D3450
Summary:
See discussion on T1630.
extraData provides more scope for extensions to piggy-back
more data on the test results and have that pulled up to the UI.
We're using keys like "facebook:complexity" to store additional
data as part of the test results.
Test Plan:
Nothing in the codebase touches extraData at the moment, so
you'll just have to have faith/prove by inspection.
Reviewers: vrana, nh, tuomaspelkonen, epriestley
Reviewed By: epriestley
CC: aran, epriestley
Maniphest Tasks: T1630
Differential Revision: https://secure.phabricator.com/D3276
Summary:
According to @epriestley, it's nasty and kind of crazy: D2933#1.
It also stands in my way for D2614.
Test Plan: Rewrote our callsite to event listener and verified that it still works.
Reviewers: tuomaspelkonen, epriestley
Reviewed By: epriestley
CC: aran, epriestley
Differential Revision: https://secure.phabricator.com/D3171
Test Plan: Show Full Unit Results on this diff.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D2777
Summary:
I just hope that we will not create `ArcanistPhutilTestCaseTestCaseTestCase`.
Also fix a copy/paste error (@edward's this time).
Test Plan: `arc unit`
Reviewers: epriestley
Reviewed By: epriestley
CC: edward, aran, Korvin
Differential Revision: https://secure.phabricator.com/D2692
Summary: Almost revert D2673 but leave the support for 'repeat'.
Test Plan: `arc help unit`.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D2689
Summary:
We run all tests before deploy but some of them are expected to fail.
We need to skip them.
It can be useful also for someone else.
I don't propagate this to `arc diff` as that would be much more complicated - we would need a new state 'partially skipped'.
The 'path' argument needs to be a file, skipping whole dirs is not supported.
The 'path' argument is in fact engine specific and engines can support format like 'Class::testMethod' but I didn't bother to document it to not confuse people.
Test Plan:
`arc unit --skip test.php`
`arc unit --skip ../src/test.php`
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D2673
Summary:
`$this->getPaths()` gives us paths relative to repository root.
We resolve them relative to cwd.
Test Plan: [src] `arc unit difference`
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Koolvin
Differential Revision: https://secure.phabricator.com/D2634
Summary:
Move away from setModule(), to setPathPrefix(). Also simplify test location/selection.
Depends on D2621.
Test Plan: Ran "arc unit".
Reviewers: vrana, btrahan
Reviewed By: vrana
CC: aran
Maniphest Tasks: T1103
Differential Revision: https://secure.phabricator.com/D2622
Summary: Mechanical changes from D2588. No "Class.php" moves yet because they aren't necessary for libraries to function.
Test Plan: See D2588.
Reviewers: vrana, btrahan, jungejason
Reviewed By: vrana
CC: aran
Maniphest Tasks: T1103
Differential Revision: https://secure.phabricator.com/D2589
Summary:
More & more use cases come up with empty suite names, guessing
and making test names nice is mess. Will leave it as it is, except
data set part, as it could be super long.
Test Plan: - Try running some phpunit tests with & without data sets
Reviewers: epriestley
CC: aran, Koolvin
Differential Revision: https://secure.phabricator.com/D2544
Summary:
Better test name handling for tests with data sets.
Instead of showing test name with full data set, show it's id/name,
e.g. `testConstructor with data set #1`
Test Plan: - Try running tests with data sets
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Koolvin
Differential Revision: https://secure.phabricator.com/D2535
Summary:
PHPUnit wrapper for arc.
The idea here is simple - find the test case which is related to the updated
class file. Generate tmp files for json & clover reports, run phpunit
with provided arguments.
It supports phpunit configuration file setting in `.arcconfig`: `phpunit_config`.
Path should be relative to project root.
Test Plan:
- Set `unit_engine` to `PhpunitTestEngine`
- Try running tests with & without `phpunit_config` option.
Reviewers: epriestley, davidreuss
Reviewed By: epriestley
CC: aran, Koolvin, jungejason
Differential Revision: https://secure.phabricator.com/D2472
Summary:
Wrapper for Python 'nose' (http://readthedocs.org/docs/nose/en/latest/)
testing tool.
Test Plan:
Install latest 'nose' v1.1.3. Currently it is available through
Github only (``pep install git+https://github.com/nose-devs/nose.git``).
Create a Python project with following structure:
/package_name/module_name.py
/tests/package_name/test_module_name.py
Write some tests
Run ``arc unit``
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Koolvin, zeeg
Differential Revision: https://secure.phabricator.com/D2322
Summary: Currently, if you change a symlink outside a libphutil library and the link target is something inside a libphutil library, we may enter an inifite loop in the "do { ... } while(...)" later. Just bail if the loop won't resolve.
Test Plan: Ran arc unit, Airtime reported the issue resolved by a similar fix.
Reviewers: cpiro, btrahan
Reviewed By: cpiro
CC: aran
Differential Revision: https://secure.phabricator.com/D2318
Summary:
Allow tests to be skipped by calling assertSkipped(). It's not really
an assertion of anything tangible; more like "assert that we can't
really assert anything right now".
Test Plan: Added a new test to the PhutilUnitTestEngineTestCase.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Koolvin
Differential Revision: https://secure.phabricator.com/D2312
Summary:
Don't use abstract subclasses of ArcanistPhutilTestCase, only use
concrete ones.
This lets you put common functionality in an abstract BaseTestCase
(which itself is a subclass of ArcanistPhutilTestCase), then implement
concrete subclasses of the BaseTestCase.
Test Plan: Tested with a simple Base -> {Case1, Case2} setup.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Koolvin
Differential Revision: https://secure.phabricator.com/D2300
Summary:
Most setters returns `$this` but some don't.
I guess it's not by purpose.
Test Plan:
arc lint
Reviewers: epriestley
Reviewed By: epriestley
CC: aran
Differential Revision: https://secure.phabricator.com/D2084
Summary: When tests have a lot of output, show a diff of the expected/actual.
Test Plan: Used this when developing D2016 to examine large output usefully.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran, epriestley
Differential Revision: https://secure.phabricator.com/D2017
Summary: We incorrectly merge array() into empty string, which is later interpreted as "this file is entirely not-executable". Instead, show no coverage information in the UI.
Test Plan: Looked at a README diff with no coverage information, got no UI render.
Reviewers: tuomaspelkonen, btrahan, zeeg
Reviewed By: tuomaspelkonen
CC: aran, epriestley
Maniphest Tasks: T965
Differential Revision: https://secure.phabricator.com/D1863
Summary: Simpler assert function for asserting a type of exception was raised.
Test Plan: Wrote this for (and tested it with) D1836.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, epriestley
Differential Revision: https://secure.phabricator.com/D1837
Summary:
We currently raise a very confusing error when we hit this case:
Exception: The phutil library '' has not been loaded!
Because of the trickiness of init-order stuff, it's difficult to detect this more explicitly earlier -- instead, just raise a more descriptive error.
Test Plan: Ran "arc unit" in a copy of libphutil other than the one arc loads.
Reviewers: btrahan, jungejason
Reviewed By: btrahan
CC: aran, epriestley
Maniphest Tasks: T580
Differential Revision: https://secure.phabricator.com/D1760
Summary:
Mark all applicable Arcanist classes as "final", except PhutilLintEngine, which
needs a little finesse.
@jungejason / @nh, does this break any Facebook stuff?
Test Plan: Linter no longer raises warnings. Ran "testEverythingImplemented" in
Phabricator.
Reviewers: btrahan, jungejason, nh
Reviewed By: btrahan
CC: aran, epriestley
Maniphest Tasks: T795
Differential Revision: https://secure.phabricator.com/D1519
Summary:
- Show file/line so you can tell which assertion failed if there's a block
with a zillion of them and they don't have messages.
- Try to format stuff a little better.
Test Plan: - Ran some failing unit tests.
Reviewers: btrahan, jungejason
Reviewed By: btrahan
CC: aran, btrahan
Differential Revision: https://secure.phabricator.com/D1304
Summary: If you modify a file at the root level (like the celerity map) we run
the dir/path checks in the wrong order and fail to detect that we can't possibly
find any modules. This leads to an infinite loop inside the while loop below.
Test Plan: Ran "arc unit" on a change which affects the celerity map.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran, btrahan
Differential Revision: 1146
modules
Summary:
Right now, if you add a file to "__tests__/data/blah.testcase" it doesn't
trigger those __tests__, but it should. Trigger __tests__ in all parent modules
when a file changes.
(We could be a little more complex about choosing what to run but all the tests
are super fast so it hardly matters if we run "too many" tests.)
Test Plan: Ran "arc unit" on this, got a test trigger. Added a file to a
__tests__/data/ directory and got a test trigger. Added some var_dump() and
spot-checked things for sanity.
Reviewers: btrahan, jungejason, nh, tuomaspelkonen, aran
Reviewed By: jungejason
CC: aran, jungejason
Differential Revision: 1127
Summary:
When a unit test throws an exception, provide more data (type, trace) in the
test failure message.
Previously, we would show only the message itself, which may not be very useful
in debugging test failures.
Test Plan: Ran "arc unit" on a test which throws, got a stack trace.
Reviewers: edward, btrahan, jungejason, nh, tuomaspelkonen, aran
Reviewed By: nh
CC: aran, nh
Differential Revision: 1123
Summary:
Keeping unit tests speedy keeps them useful since people actually won't mind
running them. This diff records the time taken by each test and displays it nice
and colorized. Really, I just want to discourage non-unit tests from making
their way into ##__tests__##.
Some thoughts:
- The "acceptableness" times are subjective but if dependencies are properly
mocked the times seem to be ok. Integration tests that make network requests to
third-party endpoints and pull in megabytes of data will not survive. This is a
good thing.
- Fast tests get a gold star, encouraging small tests. I am sorry that the star
does not sparkle.
- There is no way for a programmer to admit that their test is going to be slow
in some cases. They will be shamed with red text for the life of their test.
- It might be confusing that fast but failing tests get green text and maybe a
gold star.
Test Plan: Ran some of the unit tests within Arcanist and libphutil. See
https://secure.phabricator.com/file/view/PHID-FILE-cdd3c94c219e0fd7470b/ for
sample output.
Reviewed By: epriestley
Reviewers: epriestley
CC: aran, epriestley
Differential Revision: 588
unit'
Summary:
We want to handle 'arc unit' and 'arc diff' differently in our test
framework.
Test Plan:
Tested that with 'arc unit' the value was set to 'unit' and with 'arc diff'
the value was set to 'diff'.
Reviewed By: epriestley
Reviewers: slawekbiel, epriestley
CC: jungejason, grglr, aran, tuomaspelkonen, epriestley
Differential Revision: 430
Summary:
We changed our Facebook implementation to echo test results while the
tests are running. We do not want to echo the test results twice.
Test Plan:
Tested that implementing the function in PhutilUnitTestEngine and
returning true showed the results and returning false didn't show the
results.
Reviewed By: epriestley
Reviewers: jungejason, epriestley, grglr, slawekbiel
Commenters: slawekbiel, aran
CC: sgrimm, slawekbiel, aran, tuomaspelkonen, epriestley
Differential Revision: 400
Summary:
Test Engine classes might need the differential Diff ID to be
able to attach postponed test results to diffs.
Test Plan:
Added setDifferentialDiff function to PhutilUnitTestEngine class
and made sure it was called with the correct diff ID when running
'arc diff --preview'
Reviewed By: jungejason
Reviewers: jungejason, grglr
Commenters: sgrimm
CC: epriestley, sgrimm, slawekbiel, aran, tuomaspelkonen, jungejason
Differential Revision: 395
