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

817 commits

Author SHA1 Message Date
epriestley
6f6fde84cc Improve syntax error output from "arc liberate"
Summary:
Currently, when a file has a syntax error and you run "arc liberate", the symbol analyzer will fail and throw, which will make the mapper fail and throw, which will make arc fail and throw. This gives you a stack trace pile three scripts deep which is a giant pain to analyze visually.

Instead, raise a clear message.

Test Plan: Ran "arc liberate" with a syntax error. Got useful diagnostic output instead of 30 pages of stack mess.

Reviewers: vrana, btrahan

Reviewed By: vrana

CC: aran

Differential Revision: https://secure.phabricator.com/D2659
2012-06-06 10:53:02 -07:00
vrana
52f8f49aef Use project root instead of library root in unit engine 2012-06-05 21:08:59 -07:00
epriestley
d016f81135 Add "--conduit-timeout" to arc and modernize argument parsing
Summary:
We added "repeat" to PhutilArgumentParser a while ago, so we can modernize the rest of the argument parsing (allow multiple --load-phutil-library flags).

Add a "--conduit-timeout" flag to allow users to set the timeout. See D2602.

Test Plan:
  $ arc diff --conduit-timeout=0.01
  Exception
  [cURL/28] <CURLE_OPERATION_TIMEOUTED> The request took too long to complete.
  (Run with --trace for a full exception trace.)
  $ arc diff --conduit-version 33
  Exception
  ERR-BAD-VERSION: Your 'arc' client version is '33', which is newer than the server version, '5'. Upgrade your Phabricator install.
  (Run with --trace for a full exception trace.)
  $ arc diff --conduit-uri http://derp.derp/
  Usage Exception: YOU NEED TO INSTALL A CERTIFICATE TO LOGIN TO PHABRICATOR

  You are trying to connect to 'http://derp.derp/api/' but do not have a certificate installed for this host. Run:

        $ arc install-certificate

  ...to install one.
  $ arc diff --load-phutil-library src --load-phutil-library ../phabricator/src --trace
  Loading phutil library '0' from '/INSECURE/devtools/arcanist/src'...
  Loading phutil library '1' from '/INSECURE/devtools/phabricator/src'...

Reviewers: phleet, vrana, btrahan

Reviewed By: btrahan

CC: aran

Differential Revision: https://secure.phabricator.com/D2603
2012-06-03 08:31:49 -07:00
vrana
35b60d7562 Fix comment 2012-06-02 10:09:23 -07:00
vrana
5d78869de0 Fix context for lint errors at top of file in JSON renderer 2012-06-01 23:43:21 -07:00
vrana
2a99dc8b38 Remove unused variable 2012-06-01 23:33:58 -07:00
vrana
1d9ffce1ec Define interface for lint renderers
Summary:
It makes me nervous.

Also move them to the dir.

Test Plan:
`arc lint`
`arc lint --output json`

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Koolvin

Differential Revision: https://secure.phabricator.com/D2644
2012-06-01 17:35:38 -07:00
vrana
21acd2180e Examine only HEAD in amend workflow
Summary: We don't support `git rebase -i` or other magic in `arc amend` so we can examine just HEAD which we always amend.

Test Plan: `arc amend` on a branch with two commits.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Koolvin

Differential Revision: https://secure.phabricator.com/D2643
2012-06-01 17:06:23 -07:00
epriestley
c2788c8797 Render chevrons and carets in lint output for messages without "original" text
Summary:
If a message has only "line", or "line" and "char", we don't point out the location in the context block.

When a message includes "line", show chevrons on the line.
When a message includes "line" and "char", show chevrons on the line and a caret on the next line.

Test Plan: Ran regex linters to capture line, line+char, and normal linters to capture everything. Output appeared sane.

Reviewers: csilvers, vrana, btrahan

Reviewed By: csilvers

CC: aran

Differential Revision: https://secure.phabricator.com/D2642
2012-06-01 15:53:16 -07:00
vrana
0b45ec30be Move files in Arcanist one level up
Summary:
- `kill_init.php`
- Manually change library map.
- Manually rename `/data/` test dirs.
- [src/lint/linter] `git mv base/ArcanistLinterTestCase.php __tests__/`
- `arc liberate`

Test Plan: Browse around to make sure I like it better, especially `repository/api`, and `workflow`.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Koolvin

Maniphest Tasks: T1103

Differential Revision: https://secure.phabricator.com/D2637
2012-06-01 11:56:00 -07:00
vrana
29c8ff9ddf Fix arc unit when running from non-root directory
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
2012-06-01 11:19:52 -07:00
vrana
2ba9f34f5c Simplify diff excuses
Summary:
The theory is that lint and unit errors are rare with no false positives.
Our reality is currently different.

This diff removes the question for providing explanation before actually providing it.
I hope that the wording is not confusing.

It also simplifies the code with less copy/paste errors (not me!) potential.

Test Plan:
`arc diff` on code with lint and unit errors.
Repeat with `--excuse .`.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Koolvin, meitros

Differential Revision: https://secure.phabricator.com/D2631
2012-06-01 09:07:38 -07:00
vrana
693943c1a4 Prime SVN in parallel
Summary: Same as D2629.

Test Plan: None.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Koolvin

Differential Revision: https://secure.phabricator.com/D2632
2012-06-01 08:24:45 -07:00
vrana
7148519bbc Run xhpast in parallel
Summary: I wonder if this is not by purpose?

Test Plan: Modified two files, ran `arc lint`.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Koolvin

Differential Revision: https://secure.phabricator.com/D2629
2012-05-31 15:19:35 -07:00
epriestley
9cd098ca01 Add "ArcanistSingleLintEngine" and "ArcanistScriptAndRegexLinter"
Summary:
Request from @csilvers. Allow installs to get most linter features with regexes, configuration and external scripts if they are hesitant to write phutil libraries.

  - Add "ArcanistSingleLintEngine", which implements the smallest possible engine behavior (run one linter on every path).
  - Add "ArcanistScriptAndRegexLinter", which uses a script and a regex to parse lint output from other scripts.

Depends on D2618.

Test Plan:
Basics:

  $ arc set-config lint.engine ArcanistSingleLintEngine
  Set key 'lint.engine' = 'ArcanistSingleLintEngine'.
  $ arc set-config lint.engine.single.linter ArcanistScriptAndRegexLinter
  Set key 'lint.engine.single.linter' = 'ArcanistScriptAndRegexLinter'.
  $ arc set-config linter.scriptandregex.script 'echo derp #'
  Set key 'linter.scriptandregex.script' = 'echo derp #'.
  $ arc set-config linter.scriptandregex.regex '/^(?P<message>.*)$/m'
  Set key 'linter.scriptandregex.regex' = '/^(?P<message>.*)$/m'.
  $ arc lint
  >>> Lint for .arcconfig:

     Error  (S&RX) Lint
      derp

                 1 {
                 2   "project_id" : "arcanist",
                 3   "conduit_uri" : "https://secure.phabricator.com/",

Throw:

  $ arc set-config linter.scriptandregex.regex '/^(?P<throw>.*)$/m'
  Set key 'linter.scriptandregex.regex' = '/^(?P<throw>.*)$/m' (was '/^(?P<message>.*)$/m').
  $ arc lint
  Usage Exception: ArcanistScriptAndRegexLinter: configuration captured a 'throw' named capturing group, 'derp'. Script output:
  derp

Ignore:

  $ arc set-config linter.scriptandregex.regex '/^(?P<ignore>.*)$/m'
  Set key 'linter.scriptandregex.regex' = '/^(?P<ignore>.*)$/m' (was '/^(?P<throw>.*)$/m').
  $ arc lint
   OKAY  No lint warnings.

Severity:

  $ arc set-config linter.scriptandregex.regex '/^(?P<warning>.)(?P<message>.*)$/m'
  Set key 'linter.scriptandregex.regex' = '/^(?P<warning>.)(?P<message>.*)$/m' (was '/^(?P<ignore>.*)$/m').
  $ arc lint
  >>> Lint for src/lint/engine/single/ArcanistSingleLintEngine.php:

     Warning  (S&RX) Lint
      erp

                 1 <?php
                 2
                 3 /*

Reviewers: csilvers, btrahan, vrana

Reviewed By: csilvers

CC: aran

Differential Revision: https://secure.phabricator.com/D2619
2012-05-31 12:09:01 -07:00
epriestley
8b7a5157f8 Allow global config to load libraries and set test engines
Summary:
Khan Academy is looking into lint configuration, but doesn't use ".arcconfig" because they have a large number of repositories. Making configuration more flexible generally gives us more options for onboarding installs.

  - Currently, only project config (".arcconfig") can load libraries. Allow user config ("~/.arcrc") to load libraries as well.
  - Currently, only project config can set lint/unit engines. Allow user config to set default lint/unit engines.
  - Add some type checking to "arc set-config".
  - Add "arc set-config --show".

Test Plan:
  - **load**
    - Ran `arc set-config load xxx`, got error about format.
    - Ran `arc set-config load ["apple"]`, got warning on running 'arc' commands (no such library) but was able to run 'arc set-config' again to clear it.
    - Ran `arc set-config load ["/path/to/a/lib/src/"]`, worked.
    - Ran `arc list --trace`, verified my library loaded in addition to `.arcconfig` libraries.
    - Ran `arc list --load-phutil-library=xxx --trace`, verified only that library loaded.
    - Ran `arc list --trace --load-phutil-library=apple --trace`, got hard error about bad library.
    - Set `.arcconfig` to point at a bad library, verified hard error.
  - **lint.engine** / **unit.engine**
    - Removed lint engine from `.arcconfig`, ran "arc lint", got a run with specified engine.
    - Removed unit engine from `.arcconfig`, ran "arc unit", got a run with specified engine.
  - **--show**
    - Ran `arc set-config --show`.
  - **misc**
    - Ran `arc get-config`.

Reviewers: csilvers, btrahan, vrana

Reviewed By: csilvers

CC: aran

Differential Revision: https://secure.phabricator.com/D2618
2012-05-31 11:41:39 -07:00
epriestley
e214ea7b18 Run phutil unit tests in v2 libraries
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
2012-05-31 10:40:35 -07:00
epriestley
d80c4f683e Use ArcanistPhutilLibraryLinter, not ArcanistPhutilModuleLinter
Summary:
  - The library linter works fine on v1 or v2 libraries, so switch to it for all libraries.
  - "arc liberate" on v1 libraries will still invoke the Module linter, and thus generate __init__.php files.

Test Plan: Ran "arc lint". Grepped for module linter.

Reviewers: vrana, btrahan

Reviewed By: vrana

CC: aran

Maniphest Tasks: T1103

Differential Revision: https://secure.phabricator.com/D2620
2012-05-31 10:36:00 -07:00
vrana
5aef938391 Remove unnecessary usages of newv()
Test Plan:
  arc lint

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Koolvin

Differential Revision: https://secure.phabricator.com/D2611
2012-05-30 16:11:27 -07:00
vrana
f288ee0e21 Depend on autoloading
Test Plan:
  arc lint

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Koolvin

Differential Revision: https://secure.phabricator.com/D2609
2012-05-30 16:02:36 -07:00
vrana
94074cbc8c Revert D2306
Summary: We don't use this concept.

Test Plan:
  arc lint

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Koolvin

Maniphest Tasks: T1103

Differential Revision: https://secure.phabricator.com/D2607
2012-05-30 15:20:25 -07:00
epriestley
71afde1988 Upgrade arcanist to libphutil v2
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
2012-05-30 14:22:59 -07:00
epriestley
a8ddec4f64 Allow "arc liberate" to liberate v2 libraries and upgrade v1 -> v2
Summary:
  - "arc liberate" now works for v1 or v2 libraries.
  - "arc liberate --upgrade" converts a v1 to a v2.
  - We delete __init__.php files but do not move Class.php files, since this is vastly simpler. Authors can do this on their own if they want. We'll do it separately.
  - v2 has less lint support than v1, but I think we can move first and migrate lint support later. Much of the v1 lint is bad anyway.

Test Plan: Upgraded libphutil, arcanist and phabricator to v2

Reviewers: vrana, btrahan, jungejason

Reviewed By: vrana

CC: aran

Maniphest Tasks: T1103

Differential Revision: https://secure.phabricator.com/D2588
2012-05-30 14:18:11 -07:00
epriestley
7a4c97f9c3 Recover from all file upload exceptions; add "--skip-binaries" flag
Summary: We recover from Conduit exceptions, but not from transport exceptions or other general classes of error here. Recover from everything, and add an explicit flag to skip uploads.

Test Plan: Added a "throw", created a diff, skiped upload. Removed throw, created a diff with --skip-binaries. Created a diff normally.

Reviewers: btrahan, vrana

Reviewed By: vrana

CC: aran

Differential Revision: https://secure.phabricator.com/D2598
2012-05-30 14:11:07 -07:00
epriestley
7ae1a0ec40 Minor, address feedback from @vrana on D2585.
Auditors: vrana
2012-05-30 07:33:46 -07:00
epriestley
009e6c4dbf Add "ArcanistPhutilLibraryLinter" to replace "ArcanistPhutilModuleLinter"
Summary:
Adds a linter for v2 libraries which raises the relevant errors.

NOTE: Not hooked up anywhere yet, so this diff has no effect.

Test Plan:
Switched the ModuleLinter to LibraryLinter and ran it with a junk block to trigger errors:

  >>> Lint for src/lint/linter/phutillibrary/ArcanistPhutilLibraryLinter.php:

     Error  (PHL3) One Class Per File
      File 'lint/linter/phutillibrary/ArcanistPhutilLibraryLinter.php' mixes
      function (id) and class/interface (ArcanistPhutilLibraryLinter)
      definitions in the same file. A file which declares a class or an
      interface MUST declare nothing else.

               190 }
               191
               192 if (false) {
               193   function id() { }
               194   new XYZ();
               195 }

     Error  (PHL2) Duplicate Symbol
      Definition of function 'id' in
      'lint/linter/phutillibrary/ArcanistPhutilLibraryLinter.php' in library
      'arcanist' duplicates prior definition in 'utils/utils.php' in library
      'phutil'.

               190 }
               191
               192 if (false) {
               193   function id() { }
               194   new XYZ();
               195 }

     Error  (PHL1) Unknown Symbol
      Use of unknown class 'XYZ'. This symbol is not defined in any loaded
      libphutil library.

               191
               192 if (false) {
               193   function id() { }
               194   new XYZ();
               195 }

Reviewers: vrana, btrahan

Reviewed By: vrana

CC: aran

Maniphest Tasks: T1103

Differential Revision: https://secure.phabricator.com/D2597
2012-05-30 07:25:09 -07:00
vrana
61b0cfac63 Shorten excuse prompt in arc diff
Test Plan: `arc diff` on a commit with lint error.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Koolvin

Differential Revision: https://secure.phabricator.com/D2595
2012-05-29 12:59:42 -07:00
epriestley
4eb6b97097 Export extends/implements information in the new libphutil library map
Summary:
For `PhutilSymbolLoader` queries which include `setAncestorClass()`, we need the map of which classes/interfaces things extend/implement to issue the query efficiently.

Without this map, we need to load //every// class/interface and then do `is_subclass_of()`. This is doable, but not very performant if we don't have php-fpm warmup. There are a few performance-sensitive interfaces where we run queries like this, including some in Arcanist, where we'll never have warmup.

This map isn't particularly difficult to generate or maintain, so just include it in symbol generation and in the library map.

Also set parallelism with a flag, since it was arbitrarily hard-coded and adding flags is easy. 8 actually seems approximately optimal on my machine at least, though.

Test Plan: Ran "phutil_rebuild_map.php", opened library map, got a reasonable extension map.

Reviewers: vrana, btrahan

Reviewed By: vrana

CC: aran

Maniphest Tasks: T1103

Differential Revision: https://secure.phabricator.com/D2585
2012-05-29 11:17:34 -07:00
epriestley
e9a6cd26fc Provide a simpler library map rebuild script
Summary:
Modernize `phutil_mapper.php` to prepare for killing `__init__.php`.

The current mapper is module-oriented and complex. Instead, make the mapper file-oriented and simpler.

We build a one-to-one cache of file content to symbols (built with `phutil_symbols.php`) and then write a simpler map. See some discussion in D2561.

Also make the script less messy/bad in general. It may be useful to compare this to phutil_mapper.php.

(Additionally, we now write versions into the library map and cache.)

NOTE: Nothing can read this new map right now, of course.

Test Plan: Ran "phutil_rebuild_map.php src/" in phabricator/ with --quiet, --drop-caches, etc. Verified cache file, cache behavior, and generated map output.

Reviewers: vrana, nh, btrahan

Reviewed By: vrana

CC: aran

Maniphest Tasks: T1103

Differential Revision: https://secure.phabricator.com/D2562
2012-05-27 12:57:27 -07:00
Nick Harper
c51590dc1e Provide api to get committed data in a hook
Summary:
We have a linter that needs to read data from the repo that isn't in the commit,
so that data isn't part of the svn transaction, thus not available in svnlook
cat --transaction. This change provides a method to get data from a committed
file.

Test Plan:
Successfully ran arc svn-hook-pre-commit with a linter that needs something
not in the transaction.

Reviewers: epriestley, jungejason, vrana

Reviewed By: epriestley

CC: aran, Koolvin

Differential Revision: https://secure.phabricator.com/D2582
2012-05-25 15:28:45 -07:00
vrana
051cbcb8e9 Use phutil_console_prompt() instead of PhutilInteractiveEditor for excuses
Summary:
One line is usually more than enough for these excuses.
The excuse is quite often the same as in past so history is very useful.
Prompting user right below the errors is better than writing them below prompt.

Test Plan:
Produce a lint error.
Provide an empty explanation.
Provide a non-empty explanation.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Koolvin

Differential Revision: https://secure.phabricator.com/D2580
2012-05-25 11:42:27 -07:00
Nick Harper
fed73b75cf Change order of include_path
Summary:
With the current order of the include_path (checking in the parent dir of
arcanist last), it is possible to load the wrong libphutil which can have
bad side effects. Instead, the first place we check in include_path should
be the parent dir of arcanist.

(The issue I ran into is that I had a checkout of libphutil in my homedir,
and I was running arc from my homedir with --load-phutil-library to load
the libraries, and since ./ is the default value for include_path, we were
loading libphutil from my homedir instead of from alongside the copy of arc
that I was running. The libphutil alongside that copy of arc worked, but my
checkout of libphutil had D2545 in it, so it was broken.)

Test Plan:
ran arc (not in my homedir) with a broken libphutil in my homedir in my homedir
and it worked (before this change it didn't)

Reviewers: jungejason, vrana, epriestley

Reviewed By: vrana

CC: aran, Koolvin

Differential Revision: https://secure.phabricator.com/D2576
2012-05-25 10:08:28 -07:00
Evan Priestley
b95aac1421 Merge pull request #34 from aurelijus/phpcs-linter
PHPCS Linter
2012-05-25 06:36:23 -07:00
vrana
3e4d1c6f19 Fix whitespace
I guess that the option does not have 'supports' to allow creating aliases working in all repos.

Auditors: beng
2012-05-25 01:59:43 -07:00
vrana
a089484042 Inform user about current branch in diff
Summary: I am trying to stop using `--verbatim` and miss this information.

Test Plan: This diff.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Koolvin

Differential Revision: https://secure.phabricator.com/D2575
2012-05-24 18:13:18 -07:00
vrana
812f7f782d Don't trim spaces in diff template
Summary:
`arc diff` adds a space after each field in template.
It is later removed for the last field.

Test Plan: This diff.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Koolvin

Differential Revision: https://secure.phabricator.com/D2564
2012-05-24 11:58:00 -07:00
epriestley
81976ff2ff Provide a simpler analyzer script for killing __init__.php
Summary:
The `phutil_analyzer.php` script currently analyzes entire modules and is fairly complex. We don't need or want this in a post-__init__.php world.

This is basically a simplified version of `phutil_analyzer.php`, which takes one file and emits symbols.

Test Plan:
```$ ./scripts/phutil_symbols.php resources/test/diverse_symbols.php
{
  "have" : {
    "function"  : {
      "f" : 348
    },
    "class"     : {
      "L"      : 308,
      "A"      : 497,
      "C"      : 509,
      "D"      : 531,
      "CLocal" : 627
    },
    "interface" : {
      "ILocal" : 593
    }
  },
  "need" : {
    "function"  : {
      "g" : 402,
      "h" : 462
    },
    "class"     : {
      "B"         : 519,
      "INonlocal" : 642,
      "U"         : 552,
      "X"         : 421,
      "V"         : 557,
      "W"         : 565,
      "P"         : 572
    },
    "interface" : {
      "IForeign" : 608
    }
  }
}```

Reviewers: vrana, nh, btrahan

Reviewed By: vrana

CC: aran

Maniphest Tasks: T1103

Differential Revision: https://secure.phabricator.com/D2561
2012-05-24 10:56:56 -07:00
Evan Priestley
ee41e9c720 Merge pull request #32 from aurelijus/phpunit-simplify-names
Simplify phpunit test names
2012-05-24 07:09:09 -07:00
Evan Priestley
35c30207bd Merge pull request #31 from aurelijus/filename-linter-windows
Filename linter fix for Windows
2012-05-24 07:05:08 -07:00
Aurelijus
1c6f66dab3 PHPCS Linter
Summary:
Simple wrapper for PHP_CodeSniffer.
You need to have PHP_CodeSniffer and it's dependencies installed.

Test Plan: - Try running it with your custom lint engine

Reviewers: epriestley

CC: aran, Koolvin

Differential Revision: https://secure.phabricator.com/D2560
2012-05-24 10:55:46 +02:00
Aurelijus
c5b7c13c7c Filename linter fix for Windows
Summary: On windows paths are separated with \.

Test Plan: - Run filename linter on windows (or some path with \)

Reviewers: epriestley

CC: aran, Koolvin

Differential Revision: https://secure.phabricator.com/D2559
2012-05-24 10:28:11 +02:00
epriestley
fb4f3c9446 Allow users to define shell aliases in arc with "!"
Summary: Git supports this feature (https://git.wiki.kernel.org/index.php/Aliases), it's fairly simple to implement, and gives us a little more ammunition to dissuade users with unusual workflow requirements from actually requesting features.

Test Plan: Defined the alias described in the documentation. Ran "arc ls", "arc ls -alh", etc.

Reviewers: btrahan, csilvers

Reviewed By: csilvers

CC: aran

Differential Revision: https://secure.phabricator.com/D2552
2012-05-23 17:52:37 -07:00
Jason Ge
ed7034ab38 Fix arc ArcanistSvnHookPreCommitWorkflow
Summary:
there are two problems making the ArcanistSvnHookPreCommitWorkflow not working.

  - pathExists($path) will always return false because it hasn't loaded the path yet. Because of this, PhutilLintEngine will unset the path at https://secure.phabricator.com/diffusion/ARC/browse/master/src/lint/engine/phutil/PhutilLintEngine.php;032b9b30b0721c3f$46, so ArcanistSvnHookPreCommitWorkflow will have no path to lint against and never detects a problem.
  - In ArcanistSubversionHookAPI::getCurrentFileData(), the $path is already the full path in svnroot (path got from https://secure.phabricator.com/diffusion/ARC/browse/master/src/workflow/svn-hook-pre-commit/ArcanistSvnHookPreCommitWorkflow.php;032b9b30b0721c3f$72). Prepending the project root to it will make the file path to be wrong so that the file content is empty. Again, ArcanistSvnHookPreCommitWorkflow never detects a problem.

Test Plan:
changed --transaction to --revision in related code and manually ran the following command. It detected the syntaxt error correctly. 558937 is a revision with syntax error.

/usr/local/bin/php -f /home/jungejason/nfs_devtools/arcanist/bin/arc svn-hook-pre-commit     --load-phutil-library=/tmp/testwww/www/lib/arcanist --load-phutil-library=/home/jungejason/nfs_devtools/facebook/src /svnroot 558937 2>&1

Reviewers: epriestley, nh

Reviewed By: nh

CC: aran, Girish, Koolvin

Differential Revision: https://secure.phabricator.com/D2485
2012-05-23 16:29:27 -07:00
Nick Harper
2c72779c7d Improve example linter to not lint deleted paths
Summary:
It's easy to forget to do this when writing a new lint engine (like I recently
just did), so I added it to the example to improve documentation on how to write
a lint engine. This code is copied from PhutilLintEngine (as well as many
others).

Test Plan: php -l

Reviewers: jungejason, vrana, epriestley

Reviewed By: epriestley

CC: aran, Koolvin

Differential Revision: https://secure.phabricator.com/D2553
2012-05-23 14:59:31 -07:00
epriestley
833ae9c152 Remind user about 'arc amend' when they try to land a branch onto itself
Summary: If you develop in "master" and try to "arc land", you get an error. You can reasonably "arc amend" in this situation, most of the time. Give the user an explicit pointer.

Test Plan:
  $ arc land master
  Usage Exception: You can not land a branch onto itself -- you are trying to
  land 'master' onto 'master'. For more information on how to push changes, see
  'Pushing and Closing Revisions' in 'Arcanist User Guide: arc diff' in the
  documentation. You may be able to 'arc amend' instead.

Reviewers: csilvers, btrahan

Reviewed By: csilvers

CC: aran

Differential Revision: https://secure.phabricator.com/D2547
2012-05-23 11:55:30 -07:00
Aurelijus
2f3f45614b Simplify phpunit test names
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
2012-05-23 11:55:01 +02:00
epriestley
833e8355d8 Add "--reviewers" and "--ccs" flags to arc diff
Summary: Request from @csilvers, whose team is alergic to $EDITOR.

Test Plan:
Adding reviewers and CCs to this diff via CLI. The initial commit message for this diff is:

  Add "--reviewers" and "--ccs" flags to arc diff

  Request from @csilvers, whose team is alergic to $EDITOR.

  Tested: Adding reviewers and CCs to this diff via CLI. The initial commit message for this diff is:
  (...infinite recursion omitted...)

Reviewers: csilvers, btrahan

Reviewed By: csilvers

CC: aran

Differential Revision: https://secure.phabricator.com/D2538
2012-05-22 14:38:53 -07:00
epriestley
04606d2833 Improve default fill of Differential messages from commits
Summary:
  - When you run "arc diff" in Mercurial, we currently give you an empty template. Instead, prefill a likely template by parsing messages, as we do for Git.
  - Unify Git and Mercurial logic for acquiring messages, since `getLocalCommitInformation()` now provides this information. This should improve Git performance, and allows us to delete some code.
  - Merge messages more intelligently. Currently, we just overwrite fields. Instead, merge arrays (like ccs, reviewers, tasks) and concatenate strings (like summary and test plan). We need to special case "title", see inline.
  - @csilvers: this is a precursor to implementing "--reviewers", etc.

Test Plan: See next post, test plan includes giant output.

Reviewers: btrahan, csilvers, Makinde, jungejason, vrana

Reviewed By: csilvers

CC: aran

Differential Revision: https://secure.phabricator.com/D2536
2012-05-22 11:08:57 -07:00
Aurelijus
6e87de7304 PHPUnit test name handling improvements
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
2012-05-22 09:13:43 -07:00
Aurelijus
4f739014ae PHPUnit test engine
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
2012-05-22 07:12:03 -07:00