1
0
Fork 0
mirror of https://we.phorge.it/source/arcanist.git synced 2024-12-04 20:52:42 +01:00
Commit graph

2385 commits

Author SHA1 Message Date
epriestley
22f6207920 Fail lint tests if no tests are found in the test directory
Summary: Ref T4570. Testing a directory with no recognized tests currently passes, but should fail.

Test Plan:
  - Ran `arc unit`.
  - Removed tests from a test directory, ran `arc unit`, got test failure.

Reviewers: leebyron, btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T4570

Differential Revision: https://secure.phabricator.com/D8434
2014-03-07 10:03:17 -08:00
Peng Li
18251a1bd8 Remove a parameter
Summary: D7271 added this line but the signature of function ArcanistWorkingCopyIdentity::newFromRootAndConfigFile(...) was never changed. I have no idea why it was added here and it's causing failures in facebook. I am just removing it now.

Test Plan: No idea how to test

Reviewers: wez, lifeihuang, JoelB, #blessed_reviewers, epriestley

Reviewed By: epriestley

CC: Korvin, epriestley, aran

Differential Revision: https://secure.phabricator.com/D8432
2014-03-06 15:53:06 -08:00
epriestley
133d289ad1 Generate a documentation book for Arcanist
Summary: Ref T988. Not pretty or polished, but works fine.

Test Plan: Generated the docs, made sure they were all there.

Reviewers: chad, btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T988

Differential Revision: https://secure.phabricator.com/D8413
2014-03-05 13:00:40 -08:00
epriestley
7b6b24154b Add "Changes Planned" and "In Preparation" states for revisions
Summary:
Ref T2222. Discussion in T4481. Ref T2543.

Primarily, this will let me fix some of the rough edges that came out of ApplicationTransactions and state-based transitions.

I'm also adding a constant for T2543 while I'm in here.

Test Plan: Used `grep` to look for any weird special behavior, didn't see any.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T2543, T2222

Differential Revision: https://secure.phabricator.com/D8400
2014-03-05 10:44:26 -08:00
Joshua Spence
fb826bbf9c Correctly identify @todo comments as TODOs.
Summary: Allow `@todo` comments to be linted as TODOs as well as `TODO` comments.

Test Plan: I added a new test case (`todo.lint-test`)

Reviewers: #blessed_reviewers, epriestley

Reviewed By: epriestley

CC: Korvin, epriestley, aran

Differential Revision: https://secure.phabricator.com/D8389
2014-03-04 11:03:09 -08:00
Joshua Spence
0888b6616c Use PHP type hinting in ArcanistXHPASTLinter.
Summary: Explicitly specify the types of the function parameters.

Test Plan: `arc unit`

Reviewers: #blessed_reviewers, epriestley

Reviewed By: epriestley

CC: Korvin, epriestley, aran

Differential Revision: https://secure.phabricator.com/D8388
2014-03-04 11:02:18 -08:00
Joshua Spence
9bca4bfda8 Simplified the logic in various functions.
Summary:
- Tidied up `ArcanistCSSLintLinter::getDefaultBinary`.
- Tidied up `CSSLintLinter::getDefaultFlags` function.
- Tidied up `ArcanistPhpcsLinter::getDefaultBinary` function.
- Tidied up `ArcanistPEP8Linter::getDefaultFlags` function
- Tidied up `ArcanistFlake8Linter::getDefaultFlags`.
- Tidied up `ArcanistCppcheckLinter::getLintOptions`.
- Tidied up `ArcanistCppcheckLinter::getLintPath`.
- Tidied up `ArcanistCpplintLinter::getLintOptions`.
- Tidied up `ArcanistCpplintLinter::getLintPath`.
- Removed child functions which are identical to the corresponding parent functions.

Test Plan: `arc lint` and `arc unit`.

Reviewers: #blessed_reviewers, epriestley

Reviewed By: epriestley

CC: chad, Korvin, epriestley, aran

Differential Revision: https://secure.phabricator.com/D8385
2014-03-04 11:00:01 -08:00
Joshua Spence
9fb2a147f9 Made ArcanistNoLintTestCaseMisnamed final.
Summary: Self explanatory.

Test Plan: `arc unit`

Reviewers: #blessed_reviewers, epriestley

Reviewed By: epriestley

CC: Korvin, epriestley, aran

Differential Revision: https://secure.phabricator.com/D8386
2014-03-04 10:55:00 -08:00
Joshua Spence
53492fd01e Removed useless comments.
Summary: To me, it seems that these comments add no value. Personal opinion I suppose.

Test Plan: N/A

Reviewers: #blessed_reviewers, epriestley

Reviewed By: epriestley

CC: chad, Korvin, epriestley, aran

Differential Revision: https://secure.phabricator.com/D8382
2014-03-04 10:53:17 -08:00
Joshua Spence
9fbf316d38 Various linter fixes
Summary:
Fixed various linter issues that were identified by running `arc lint --everything`.

- Removed trailing whitespace.
- Added newline at EOF.
- Added whitespace after `if` statement

Test Plan: N/A

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley

CC: aurelijus, Korvin, epriestley, aran

Differential Revision: https://secure.phabricator.com/D8337
2014-02-25 07:53:07 -08:00
epriestley
a11421ee8e Add password_* functions to extension functions list
Summary: These are technically added in PHP 5.5.0 mainline, not an extension, but the two are effectively equiavlent.

Test Plan: o.O

Reviewers: btrahan, dctrwatson

Reviewed By: btrahan

CC: aran

Differential Revision: https://secure.phabricator.com/D8280
2014-02-20 17:26:03 -08:00
Joshua Spence
bd6cc6b946 Update CSSLint bindings for v0.10.0
Summary: See <https://github.com/facebook/arcanist/pull/131>

It looks like the behaviour of csslint has changed since the ArcanistCSSLintLinter was written. Consequently, ArcanistCSSLintLinter does not work with the latest version of csslint (v0.10.0).

  - `csslint` has a non-zero exit status
  - Fixed `csslint` parsing for v0.10.0

Reviewed by: epriestley
2014-02-16 15:20:29 -08:00
epriestley
13fd0a5ef1 Minor modernizations to arc browse
Summary:
Do a little cleanup:

  - Remove copyright header (we removed all of these a long time ago, this one just snuck through somehow).
  - Remove `@group` comment (obsolete with new Diviner).
  - Note support for all VCSes.
  - Add pht() for translation.
  - Hint `arc browse .`.
  - Fail on no paths sooner.
  - Raise a useful error if we can't figure out which repository we're heading to.
  - Clarify "open" comment.
  - Use `Filesystem::binaryExists()`.
  - Some minor wordsmithing.

Test Plan: `arc browse`, `arc browse .`, `arc browse README`, `arc browse README src`, ran `arc browse` in valid working copy with no associated repo.

Reviewers: btrahan, spicyj

Reviewed By: spicyj

CC: aran

Differential Revision: https://secure.phabricator.com/D8176
2014-02-09 12:19:57 -08:00
Ben Alpert
52d3cd1b4e Support browsing '.' and default to it
Test Plan: Ran 'arc browse', 'arc browse .', 'arc browse main.py'.

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley

CC: Korvin, epriestley, aran, avivey

Differential Revision: https://secure.phabricator.com/D8174
2014-02-09 09:01:52 -08:00
Ben Alpert
c3fe03db08 Detect repo for 'browse'; don't require project ID
Test Plan: Ran 'arc browse' in a repo that contains a .arcconfig and one without.

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley

CC: Korvin, epriestley, aran

Differential Revision: https://secure.phabricator.com/D8173
2014-02-09 08:55:16 -08:00
epriestley
7c46000527 Permanently remove license linters
Summary: For eventual commit.

Test Plan: none

Reviewers: nh, btrahan

Reviewed By: btrahan

CC: aran, vrana, FacebookPOC

Maniphest Tasks: T2274

Differential Revision: https://secure.phabricator.com/D4907
2014-02-03 10:01:19 -08:00
Richard van Velzen
114bb9f25b Align usages of "\1" and "\2" in MercurialRepositoryAPI
Summary:
Especially on Windows it is hard to use "\1" type escapes in shell commands. The direct usage resulted in some undefined variables because the \1 and \2 weren't actually passed as control characters.

By passing them through the regular arguments list they get sent in the "correct way" regardless of OS

Test Plan: Executed `arc diff` in a HG repo and did not get undefined indexes back

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley

CC: Korvin, epriestley, aran

Differential Revision: https://secure.phabricator.com/D8129
2014-02-02 08:26:25 -08:00
epriestley
988a482ff3 Minor, fix an issue with SVN. 2014-01-29 15:05:51 -08:00
Guy Warner
14f2f7fdd2 Arc output the correct action being taken
Summary: Fixes T3929. If arc is merging it will say merging * into * vs always saying rebasing * onto *

Test Plan: Make arc do a rebase, then a merge

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley

CC: Korvin, epriestley, aran

Maniphest Tasks: T3929

Differential Revision: https://secure.phabricator.com/D8085
2014-01-28 08:54:01 -08:00
epriestley
d1a68ccb37 Don't consider a no-op amend to be an error in Mercurial
Summary:
Unlike git, Mercurial considers an `hg commit --amend` which doesn't change the working copy to be an error.

The current behavior for this is fairly bad, since the user gets an exception wrapping the entire command ("Command Failed! ...") and it's pretty verbose and not obvious what has happened.

Some alternatives are:

  1. Detect this condition and raise a more tailored exception, like a UsageException.
  2. Detect this condition and succeed.

Although I tend to think (1) is the right approach in general (that is, `arc x` should usually behave like `git x` or `hg x`), I went with (2) here because we have a handful of amend callsites and they all assume git semantics (no-op amends are successful), and because I think Mercurial's behavior is a little silly (the working copy ends up in the correct / expected state, which seems fairly clearly like a success to me).

Test Plan:
  - Had reporting user verify patch.
  - Ran `arc amend --revision Dxxx` twice in a Mercurial working copy.

The old output looked like this:

  $ arc amend --revision 922
  Amending commit message to reflect revision D922: Improve performance of PhutilSymbolLoader::selectAndLoadSymbols().
  Exception
  Command failed with error #1!
  COMMAND
  HGPLAIN=1 hg commit --amend -l '/private/var/folders/8k/c3vkmjy5335gcxdzxkhwq82w0000gn/T/71k8q057844c4g84/3539-gfkvV4'

  STDOUT
  nothing changed

  STDERR
  (empty)
  (Run with --trace for a full exception trace.)

The new output looks like this:

  $ arc amend --revision 922
  Amending commit message to reflect revision D922: Improve performance of PhutilSymbolLoader::selectAndLoadSymbols().
  Done.

Reviewers: btrahan, richardvanvelzen

Reviewed By: richardvanvelzen

CC: aran

Differential Revision: https://secure.phabricator.com/D8083
2014-01-28 03:03:30 -08:00
epriestley
7116659289 Improve some messaging in arc which for autodetection of repositories
Summary:
  - The modern name for the config is "project.name".
  - Missing parameter in a pht().
  - When the value is set, but not valid, we gave you a misleading error message.

Test Plan: Ran `arc which`.

Reviewers: talshiri, btrahan

Reviewed By: talshiri

CC: aran

Differential Revision: https://secure.phabricator.com/D8081
2014-01-27 19:45:18 -08:00
epriestley
1daa719ace Don't choke on blame of files with whitespace-only trailing lines
Summary:
Fixes T4349. Two issues:

  - As discussed in T4349, we would trim the entire output and then require spaces when matching. This choked incorrectly if the last line of a file contained only whitespace. Use `phutil_split_lines()` instead, and regexp things more reasonably.
  - We were capturing the line text, not the commit, as "revision". This isn't actually used elsewhere, but was obviously wrong. Make this consistent with Git/SVN.

Test Plan: Rigged a call up and saw reasonable output after the patch, on a working copy which threw before the patch.

Reviewers: durham, btrahan

Reviewed By: durham

CC: aran

Maniphest Tasks: T4349

Differential Revision: https://secure.phabricator.com/D8078
2014-01-27 11:03:08 -08:00
epriestley
b1d3948d77 Default to array() when failing to look up repository PHIDs.
Auditors: btrahan
2014-01-27 10:36:40 -08:00
epriestley
2ef2b76f10 Restore "project.name" (previously "project_id") to .arcconfig for arcanist
Summary:
A couple of things actually still care about this:

  - The code that detects if we're running `arc` on `arc`, and then runs the `arc` we're running on instead of the `arc` we ran, checks for the project being "arcanist" (`reenter_if_this_is_arcanist_or_libphutil()`).
  - Some of the symbol stuff still relies on it.
  - Removing it was really more to prove it works (which it seems to) than to accomplish any concrete goals.

Test Plan: looked at the file I guess

Reviewers: btrahan, talshiri

Reviewed By: talshiri

CC: aran

Differential Revision: https://secure.phabricator.com/D8076
2014-01-26 16:11:25 -08:00
epriestley
a7376624b4 Allow arc to identify repositories without "project_id"
Summary:
Ref T4343. Continues the process of reducing the prominence of Arcanist Projects. Primarily:

  - Query Phabricator to identify the working copy based on explicit configuration, or guess based on heuristics.
  - Enhance `arc which` to explain the process to the user.
  - The `project_id` key is no longer required in `.arcconfig`.

Minor/cleanup changes:

  - Rename `project_id` to `project.name` (consistency, clarity).
  - Rename `conduit_uri` to `phabricator.uri` (consistency, clairty).
  - These both need documentation updates.
  - Add `repository.callsign` to explicitly bind to a repository.
  - Updated `.arcconfig` for the new values.
  - Fix a unit test which broke a while ago when we fixed a rare definition of "unstaged".
  - Make `getRepositoryUUID()` generic so we can get rid of one `instanceof`.

Test Plan:
  - Ran `arc which`.
  - Ran `arc diff`.
  - This doesn't really change anything, so the only real risk is version compatibility breaks. This //does// introduce such a break, but the window is very narrow: if you upgrade `arc` after this commit, and try to diff against a Phabricator which was updated after yesterday (D8068) but before D8072 lands, the lookup will work so we'll add `repositoryPHID` to the `differential.creatediff` call, but it won't exist in Phabricator yet. This window is so narrow that I'm not going to try to fix it, as I'd guess there is a significant chance that no users will be affected. I don't see a clever way to fix it that doesn't involve a lot of work, either.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T4343

Differential Revision: https://secure.phabricator.com/D8073
2014-01-26 15:31:30 -08:00
epriestley
e4b1e8e681 Use user.query, not user.find, in arc tasks
Summary: I'm going to deprecate `user.find`, `user.query` is more modern/powerful and obsoletes it.

Test Plan: Ran `arc tasks --owner epriestley`.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Differential Revision: https://secure.phabricator.com/D8070
2014-01-25 14:23:23 -08:00
Sergey Sharybin
2c2c5663cb Fix exception on diff parse on FreeBSD
FreeBSD 9.2 comes with diff tool version 2.8.7 which behaves
a bit different from how it is expected to. Namely for diff
between two binary files it says:

  Files A and B are differ

This was leading to an exception when browsing revisions with
changes in binary files.

Tweaked parse patterns in order to fix this issue. Now both
older and newer diff tools are supported.

See: <https://github.com/facebook/arcanist/pull/139>

Reviewed by: epriestley
2014-01-18 11:13:28 -08:00
epriestley
a247e21093 Call willLintPath() hook from PhutilLinter
Summary:
Some callpaths end up here, which doesn't cause the internal linter's willLintPath() method. This can mean its `activePath` is set wrong, which results in us raising lint for the wrong file.

Particularly, if you apply D7979 (diff 18069) and `arc lint` it, you'll get a syntax error message in the wrong file.

Test Plan: Applied D7979 and linted it, got proper error message.

Reviewers: btrahan

Reviewed By: btrahan

CC: aarwine, aran

Differential Revision: https://secure.phabricator.com/D7988
2014-01-17 21:30:18 -08:00
Jonathan Hitchcock
7ce0c8e84e Prompt user for commit message when auto-committing
See: <https://github.com/facebook/arcanist/pull/137>

Reviewed by: epriestley
2014-01-14 15:27:50 -08:00
Joshua Spence
d62bd48a81 Added ArcanistTextLinter::LINT_BOF_WHITESPACE and ArcanistTextLinter::LINT_EOF_WHITESPACE
Summary: This might not be universally desireable, but I found myself writing an additional linter (which I had called `WhitespaceTextLinter`) for the sake of these two linter tests. I figured it may be of use upstream, and so I decided to submit it as a diff. I won't be offended if it is rejected however.

Test Plan: `arc lint` and `arc unit` are both okay with it.

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley

CC: Korvin, epriestley, aran

Differential Revision: https://secure.phabricator.com/D7957
2014-01-13 18:05:42 -08:00
Joshua Spence
dd12b72f9a Added test namespace field to ArcanistUnitTestResult
Summary: See https://github.com/facebook/libphutil/issues/34

Test Plan: Run `arc unit`

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley

CC: Korvin, epriestley, aran

Differential Revision: https://secure.phabricator.com/D7952
2014-01-13 12:37:26 -08:00
Joshua Spence
ef598794a8 Skip unit tests if ArcanistLinter::getCacheVersion throws an ArcanistUsageException
Summary: Fixes T4288

Test Plan: N/A

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley

CC: Korvin, epriestley, aran

Maniphest Tasks: T4288

Differential Revision: https://secure.phabricator.com/D7913
2014-01-09 09:25:35 -08:00
Joshua Spence
66e3614f69 Allow PhutilUnitTestEngine::getTestsForPaths to return paths from the library root directory
Summary: Currently, running `arc unit -- src/` returns with `No tests to run`, even if there are test classes in `src/__tests__/`. This diff changes this behaviour so that `arc unit -- src/` executes unit tests in `src/__tests__/`.

Test Plan: N/A. I suppose you could create a file `src/__tests__/SomeTestCase.php` and see for yourself.

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley

CC: Korvin, epriestley, aran

Differential Revision: https://secure.phabricator.com/D7915
2014-01-09 05:21:55 -08:00
Joshua Spence
e2234a5be9 Allow ArcanistLinterTestCase to find "lint-test" files at an arbitrary depth
Summary: Self-explanatory

Test Plan: N/A

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley

CC: Korvin, epriestley, aran

Differential Revision: https://secure.phabricator.com/D7910
2014-01-08 16:23:58 -08:00
Joshua Spence
90a5a8512e Allow tests to be executed in the root library directory
See: <https://github.com/facebook/arcanist/pull/135>

Reviewed by: epriestley
2014-01-03 12:55:10 -08:00
epriestley
488b8e365a In Subversion, treat missing files similarly to conflicted files
Summary: See <https://github.com/facebook/arcanist/issues/133>. Treat "!" files like "C" and "?" files and make the user deal with them.

Test Plan:
  >>> orbital ~/repos/INIS $ svn st
  !       README
  >>> orbital ~/repos/INIS $ arc diff
  Usage Exception: You have missing files in this working copy. Revert or formally remove them (with `svn rm`) before proceeding.

    Working copy: /INSECURE/repos/INIS/

    Missing files in working copy:
      README

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Differential Revision: https://secure.phabricator.com/D7886
2014-01-03 12:23:34 -08:00
epriestley
760385ea01 Recognize ZipArchive as an extension class
Summary: See D7879 for an example. This is an extension class.

Test Plan: You do it bwahaha

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Differential Revision: https://secure.phabricator.com/D7880
2014-01-02 17:08:48 -08:00
iodragon
6769c6b17c Show current config and source using arc set-config --show
Summary:
`arc set-config --show` only show the user config
It would be better to contain local/global/system config

Test Plan: set config by local/global/system/user/project, and check the result of `arc set-config --show`

Reviewers: #blessed_reviewers, epriestley

Reviewed By: epriestley

CC: epriestley, aran

Differential Revision: https://secure.phabricator.com/D7851
2014-01-02 12:09:03 -08:00
Eric Stern
739881a3f4 Print a more useful error message if any PHPUnit tests crash
Summary:
Before, if PHPUnit crashed (syntax error, undefined constant, etc), you would get a relatively unhelpful error message:

    firehed@Eric-Sterns-Mac-Pro ~/dev/php-lcd: arc unit
    Exception
    Clover coverage XML report file is empty, it probably means that phpunit failed to run tests. Try running arc unit with --trace option and then run generated phpunit command yourself, you might get the answer.
    (Run with --trace for a full exception trace.)

This now checks the json and code coverage reports and tries to pull a useful error message:

  firehed@Eric-Sterns-Mac-Pro ~/dev/php-lcd: arc unit
  Exception
  The test '/Users/firehed/dev/php-lcd/tests/PlateButtonsTest.php' crashed with the following output:

  Fatal error: Undefined class constant 'EFT' in /Users/firehed/dev/php-lcd/tests/PlateButtonsTest.php on line 25

  Call Stack:
  	0.0002     233104   1. {main}() /Users/firehed/dev/php-lcd/vendor/phpunit/phpunit/composer/bin/phpunit:0
  	0.0039     564872   2. PHPUnit_TextUI_Command::main() /Users/firehed/dev/php-lcd/vendor/phpunit/phpunit/composer/bin/phpunit:63
  	0.0039     565496   3. PHPUnit_TextUI_Command->run() /Users/firehed/dev/php-lcd/vendor/phpunit/phpunit/PHPUnit/TextUI/Command.php:129
  	0.0247    2280168   4. PHPUnit_TextUI_TestRunner->doRun() /Users/firehed/dev/php-lcd/vendor/phpunit/phpunit/PHPUnit/TextUI/Command.php:176
  	0.0293    2730760   5. PHPUnit_Framework_TestSuite->run() /Users/firehed/dev/php-lcd/vendor/phpunit/phpunit/PHPUnit/TextUI/TestRunner.php:349
  	0.1211    3996832   6. PHPUnit_Framework_TestSuite->runTest() /Users/firehed/dev/php-lcd/vendor/phpunit/phpunit/PHPUnit/Framework/TestSuite.php:745
  	0.1211    3996832   7. PHPUnit_Framework_TestCase->run() /Users/firehed/dev/php-lcd/vendor/phpunit/phpunit/PHPUnit/Framework/TestSuite.php:775
  	0.1211    3996832   8. PHPUnit_Framework_TestResult->run() /Users/firehed/dev/php-lcd/vendor/phpunit/phpunit/PHPUnit/Framework/TestCase.php:783
  	0.1233    3999752   9. PHPUnit_Framework_TestCase->runBare() /Users/firehed/dev/php-lcd/vendor/phpunit/phpunit/PHPUnit/Framework/TestResult.php:648
  	0.1236    4016432  10. PHPUnit_Framework_TestCase->runTest() /Users/firehed/dev/php-lcd/vendor/phpunit/phpunit/PHPUnit/Framework/TestCase.php:838
  	0.1237    4017240  11. ReflectionMethod->invokeArgs() /Users/firehed/dev/php-lcd/vendor/phpunit/phpunit/PHPUnit/Framework/TestCase.php:983
  	0.1237    4017520  12. Firehed\PlateButtonsTest->testSingleButtonOnButtonDown() /Users/firehed/dev/php-lcd/vendor/phpunit/phpunit/PHPUnit/Framework/TestCase.php:983

  (Run with --trace for a full exception trace.)

Or if nothing was written to `stderr`, you may get something like this:

  firehed@Eric-Sterns-Mac-Pro ~/dev/php-lcd: arc unit --no-coverage
  Exception
  Test Firehed\PlateButtonsTest::testSingleButtonOnButtonDown did not finish
  (Run with --trace for a full exception trace.)

Test Plan:
`arc unit` for arcanist itself
`arc unit` before and after the change on a project where:
* all tests pass
* some tests are failing
* a test causes a fatal error (undefined constant, bad method call, etc)
* a test file is invalid (syntax error)

No regressions that I could find, and all crashes now display a more useful error.

Reviewers: #blessed_reviewers, epriestley

Reviewed By: epriestley

CC: aurelijus, epriestley, aran

Differential Revision: https://secure.phabricator.com/D7848
2014-01-02 12:02:22 -08:00
epriestley
35c01eee7b Improve arc lint --output summary
Summary:
This currently output like this:

  file_a:
  file_b:
  file_c:
    Warning on line 29: blah blah

This isn't especially useful and can't be piped to other tools. Instead, emit output like:

  file_c:29:Warning: blah blah

This is greppable / pipeable.

Test Plan: Ran `arc lint --output summary`.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Differential Revision: https://secure.phabricator.com/D7788
2013-12-18 14:21:03 -08:00
epriestley
8e177c4db8 Parse the output of svnlook diff ...
Summary: Ref T4195. When figuring out changed content in SVN, the easiest approach is to use `svnlook diff`, but it has a slightly different header than we're used to. Adjust the parser for it and add some tests.

Test Plan: Clean unit tests.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T4195

Differential Revision: https://secure.phabricator.com/D7790
2013-12-18 14:20:59 -08:00
Joshua Spence
cd0ee5492a Make sure the local arc config directory exists on ArcanistWorkingCopyIdentity::writeLocalArcConfig
See: <https://github.com/facebook/arcanist/pull/126>

Reviewed by: epriestley
2013-12-11 06:42:46 -08:00
epriestley
e0b4eef9de Make sure no one ever misunderstand the "unkonwn symbol" lint message ever again
Summary: A lot of new contributors don't resolve this. Make it easier to resolve, more comprehensive, and more explicit about not being ignorable.

Test Plan:
  >>> orbital ~/devtools/arcanist $ arc lint
  >>> Lint for src/lint/linter/ArcanistPhutilLibraryLinter.php:

     Error  (PHL1) Unknown Symbol
      Use of unknown class 'BlerpBarp'. Common causes are:

        - Your libphutil/ is out of date.
          This is the most common cause.
          Update this copy of libphutil: /INSECURE/devtools/libphutil

        - Some other library is out of date.
          Update the library this symbol appears in.

        - This symbol is misspelled.
          Spell the symbol name correctly.
          Symbol name spelling is case-sensitive.

        - This symbol was added recently.
          Run `arc liberate` on the library it was added to.

        - This symbol is external. Use `@phutil-external-symbol`.
          Use `grep` to find usage examples of this directive.

      *** ALTHOUGH USUALLY EASY TO FIX, THIS IS A SERIOUS ERROR.
      *** THIS ERROR IS YOUR FAULT. YOU MUST RESOLVE IT.

               181               "*** THIS ERROR IS YOUR FAULT. YOU MUST RESOLVE IT.");
               182
               183             if (false) {
      >>>      184               new BlerpBarp();
               185             }
               186           }
               187         }
  >>> orbital ~/devtools/arcanist $

Reviewers: btrahan, chad

Reviewed By: chad

CC: aran

Differential Revision: https://secure.phabricator.com/D7752
2013-12-10 08:33:35 -08:00
epriestley
4fdb87761b Fix working copy binding powers in weird edge cases
Summary:
Fixes T1277. The rules we use to figure out the root of the working copy get a bunch of edge cases wrong right now. A particularly troublesome one is when a user has a `/.arcconfig` or `/home/.arcconfig` or similar, which raises a completely useless and confusing error message (T1277).

Rewrite these rules to get all the edge cases correct and do reasonable things in the presence of stray `.arcconfig`. There are a bunch of comments, but basically the algorithm is:

  - From the top, go down one directory at a time until we find ".svn", ".git", or ".hg".
  - In Subversion, keep going down looking for ".arcconfig". In Git and Mercurial, look for ".arcconfig" only in the same directory.
  - Now that we've figured out the VCS root (where the ".vcs" directory is) and the project root (where the ".arcconfig" file is, if it exists), build an identity.

This logic was also spread across three different places. Consolidate it into one and add some logging so we can figure out what's going wrong if users run into trouble.

Test Plan:
  - Ran VCS (`arc list`) and non-VCS (`arc help`) commands in Git, Mercurial, and Subversions roots and subdirectories. Also ran them in non-VCS directories. Ran them with and without .arcconfig. All the outputs seemed completely reasonable.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T1277

Differential Revision: https://secure.phabricator.com/D7686
2013-12-03 10:32:31 -08:00
James Rhodes
00d5eb2f21 Update C# tools unit test engine to work
Summary: This updates the C# tools unit test engine (the derived version of xUnit.NET test engine that supports code coverage) to work after the updates in D7594.

Test Plan:
Switched the test engine from 'XUnitTestEngine' to 'CSharpToolsTestEngine' and ran `arc unit --everything`.  The results were identical except that code coverage was provided.

Tested on Linux; will update or comment based on whether it works on Windows as well.

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley

CC: Korvin, epriestley, aran

Differential Revision: https://secure.phabricator.com/D7640
2013-11-26 18:00:44 +11:00
Chris Dentel
d30c77d0a8 Add output-json option to arc feature
Summary:
The current format of arc feature is not very friendly to parsing.  This adds a json output
format.

Test Plan: Run the command and ensure output is valid

Reviewers: lifeihuang, #blessed_reviewers, epriestley

Reviewed By: epriestley

CC: epriestley, aran, Korvin

Differential Revision: https://secure.phabricator.com/D7647
2013-11-25 11:50:11 -08:00
James Rhodes
799841a0d8 Remove debugging output from xUnit.NET test engine
Summary: This just removes the `print "Discovered test .."` code in the xUnit.NET unit test engine.  If users need to diagnose what tests are being debugged, it can be found out with `--trace` and observing what build commands are invoked.

Test Plan: Ran `arc unit --everything` and didn't see the message any more.

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley

CC: Korvin, epriestley, aran

Differential Revision: https://secure.phabricator.com/D7639
2013-11-24 11:36:45 +11:00
James Rhodes
bd191c2860 Fix issues in C# unit test engine
Summary: This fixes a few issues in the C# unit test engine.  It fixes tests sitting in subdirectories not being tested correctly (the location of both the test assembly and the results file would be wrong).  It also fixes a very strange issue where xUnit.NET seems to not output the resulting XML file when it executes; in this case we just retry running the test until the XML file appears after completion (and eventually it works).

Test Plan: Ran `arc unit --everything` and `arc unit --everything --no-coverage` and verified that it's all reliably working.

Reviewers: epriestley, #blessed_reviewers, hach-que

Reviewed By: epriestley

CC: Korvin, epriestley, aran

Differential Revision: https://secure.phabricator.com/D7594
2013-11-22 14:32:48 -08:00
Ramkumar Ramachandra
6033f14221 ArcanistGitAPI: replace 'ls-files -m' with diff-files
As documented,

  $ git ls-files -m

fails to exclude ignored submodules. Replace it with an equivalent:

  $ git diff-files --name-only

which does not suffer from this defect.

See: <https://github.com/facebook/arcanist/pull/121>

Reviewed by: epriestley
2013-11-22 08:39:20 -08:00
Ramkumar Ramachandra
e62b23e67d ArcanistGitAPI: document a git ls-files bug
When a submodule is ignored (ignore=all in .gitconfig),

  $ git ls-files -m

fails to exclude the submodule from the listing. Other commands like

  $ git diff-index --name-only HEAD

exclude it just fine.

See: <https://github.com/facebook/arcanist/pull/120>

Reviewed by: epriestley
2013-11-22 05:35:37 -08:00