1
0
Fork 0
mirror of https://we.phorge.it/source/arcanist.git synced 2024-12-05 21:26:13 +01:00
Commit graph

2423 commits

Author SHA1 Message Date
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
Ramkumar Ramachandra
86eae809e0 ArcanistGitAPI: remove a bogus comment
See: <628de7d7a1 (commitcomment-4675543)>

This comment is indecipherable even if it originally meant something.

Reviewed by: epriestley
2013-11-22 05:28:37 -08:00
epriestley
6dade54efb Improve exception raised when an event listener fails to load
Summary: See IRC. The failing class might not be the listener itself.

Test Plan:
  $ arc list
  ERROR: Failed to load event listener 'depr': Failed to load class or interface 'depr': the class or interface 'depr' is not defined in the library map for any loaded phutil library. If this symbol was recently added or moved, your library map may be out of date. You can rebuild the map by running 'arc liberate'. For more information, see: http://www.phabricator.com/docs/phabricator/article/libphutil_Libraries_User_Guide.html

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Differential Revision: https://secure.phabricator.com/D7616
2013-11-20 12:46:38 -08:00
James Rhodes
fb8a0d32ae Set limit for execution in C# linter
Summary: This sets the limit for future execution in the C# linter to 8.  See D7606 for more information.

Test Plan:
Ran

```
arc lint --everything --trace --never-apply-patches --output json
```

and saw it only run 8 commands at once.

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley

CC: Korvin, epriestley, aran

Differential Revision: https://secure.phabricator.com/D7607
2013-11-19 13:36:27 -08:00
James Rhodes
61753b6770 Upgrade C# linter to support linting multiple files at a time
Summary:
This upgrades `cslint` to support linting multiple files at a time.  As this required a backwards-incompatible to `cslint`, I've added a SUPPORTED_VERSION constant which can be used to detect these kinds of breaking changes in the future (and prompt users to upgrade the `cslint` they have in their repository).

The reason for this upgrade is mainly around running `arc lint --everything`, where there are significant performance benefits gained when bulk linting lots of files per command execution.

Test Plan: Upgraded `cslint` in the Tychaia repository and ran `arc lint --everything --trace`.  Saw a substantially less number of executions happening for linting and all of the results came through as expected.

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley

CC: Korvin, epriestley, aran, waynea

Differential Revision: https://secure.phabricator.com/D7599
2013-11-18 11:32:58 -08:00
Burak Yigit Kaya
19edbbb46e Make ScriptAndRegexLinter available to ConfigurationDrivenLintEngine
See: <https://github.com/facebook/arcanist/pull/118>

Reviewed by: epriestley
2013-11-18 10:08:19 -08:00
Anirudh Sanjeev
17e4e9e651 Fix markup in documentation for arcanist lint engine
Summary: Fixed wrong markup which was 1 space instead of 2

Test Plan: I couldn't test it as I don't know how to build documentation

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley

CC: Korvin, epriestley, aran

Differential Revision: https://secure.phabricator.com/D7587
2013-11-14 10:19:08 -08:00
epriestley
9310df9615 Handle empty output from hg --debug branches in the parser
Summary: Ref T1493. Also consolidate this a bit more.

Test Plan: Unit tests.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T1493

Differential Revision: https://secure.phabricator.com/D7481
2013-11-04 12:15:12 -08:00
Sven Axelsson
aabbdbd2ab Filter out messages from included files
Summary:
CppCheck shows lint messages from included files as well as the current
file. Filter out those, since they don't make much sense in the context
of `arc lint`.

Test Plan:
Before this patch, `arc lint` using `ArcanistCppcheckLinter`. Note that lint messages
from included files appear pointing to a line in the active file.
After this patch, only messages from the active file are included.

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley

CC: Korvin, epriestley, aran

Differential Revision: https://secure.phabricator.com/D7444
2013-10-30 13:46:30 -07:00
Aviv Eyal
6dc04af6e8 Specify custom arcrc filename
Summary: allow specifing an replacement .arcrc file, for the odd cases.

Test Plan: `echo {} | arc call-conduit user.whoami` with and without `--arcrc-file=`

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley

CC: Korvin, aran

Differential Revision: https://secure.phabricator.com/D7208
2013-10-25 15:57:38 -07:00
Aviv Eyal
0d212ccf5a rename getConfig -> getProjectConfig, make all linters use getConfigFromAnySource
Summary:
Lingers on from D7271; Rename `ArcanistWorkingCopyIdentity.getConfig()`.

Changed all linters (Except one) to use `getConfigFromAnySource()`, because it seems to make sense.

Test Plan: arc unit --everything; arc lint in github.com:epriestley/arclint-examples.git (Except for phpcs, flake8, cpplint and csslint which I don't have installed).

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley

CC: chad, Korvin, epriestley, aran

Differential Revision: https://secure.phabricator.com/D7382
2013-10-22 15:34:06 -07:00
Aviv Eyal
4103bc423c get Config Manager to Unit engine and use getConfigFromAnySource
Summary:
somewhat related to D7271 and D7377.
Not actually broken right now, but might be worth it for completeness.

Test Plan: arc unit invokes PHPUnit; Can't test Csharp/XUnit on my end.

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley

CC: aurelijus, Korvin, epriestley, aran

Differential Revision: https://secure.phabricator.com/D7381
2013-10-22 13:58:32 -07:00
epriestley
ddbc14ade1 Provide ConfigurationManager to LintEngine in Arcanist
Summary: Unbreaks ArcanistSingleLintEngine / ArcanistScriptAndRegexLinter from recent config churn.

Test Plan: `arc lint --engine ArcanistSingleLintEngine --rev HEAD^`

Reviewers: btrahan

Reviewed By: btrahan

CC: csilvers, aran

Differential Revision: https://secure.phabricator.com/D7377
2013-10-21 16:57:22 -07:00