unit'
Summary:
We want to handle 'arc unit' and 'arc diff' differently in our test
framework.
Test Plan:
Tested that with 'arc unit' the value was set to 'unit' and with 'arc diff'
the value was set to 'diff'.
Reviewed By: epriestley
Reviewers: slawekbiel, epriestley
CC: jungejason, grglr, aran, tuomaspelkonen, epriestley
Differential Revision: 430
Summary:
Differential showed 'okay' as the arc unit status even when there were
postponed tests.
Test Plan:
Tested that test results were pushed to differential when there were
postponed tests.
Reviewed By: jungejason
Reviewers: epriestley, jungejason
CC: slawekbiel, aran, jungejason
Differential Revision: 417
Summary:
We changed our Facebook implementation to echo test results while the
tests are running. We do not want to echo the test results twice.
Test Plan:
Tested that implementing the function in PhutilUnitTestEngine and
returning true showed the results and returning false didn't show the
results.
Reviewed By: epriestley
Reviewers: jungejason, epriestley, grglr, slawekbiel
Commenters: slawekbiel, aran
CC: sgrimm, slawekbiel, aran, tuomaspelkonen, epriestley
Differential Revision: 400
Summary:
I typed the wrong number of spaces into some of these.
Test Plan:
Visual inspection, ran 'arc unit'
Reviewed By: tuomaspelkonen
Reviewers: tuomaspelkonen
CC: aran, tuomaspelkonen
Differential Revision: 402
Summary:
Test Engine classes might need the differential Diff ID to be
able to attach postponed test results to diffs.
Test Plan:
Added setDifferentialDiff function to PhutilUnitTestEngine class
and made sure it was called with the correct diff ID when running
'arc diff --preview'
Reviewed By: jungejason
Reviewers: jungejason, grglr
Commenters: sgrimm
CC: epriestley, sgrimm, slawekbiel, aran, tuomaspelkonen, jungejason
Differential Revision: 395
Summary:
There are a bunch of different ways we could approach this, but practically I
think this is probably the best one even though it's kind of yucky.
A big part of my motivation here is that if we just reject UTF-8 outright, users
are going to end up in a situation they don't understand how to resolve.
UPDATE: after discussion, going with a more conservative approach until such
time as we have a more compelling case for the less strict functionality.
Test Plan:
Created a diff with a text file that contained invalid UTF-8 subsequences.
Reviewed By: tuomaspelkonen
Reviewers: tuomaspelkonen, jungejason, aran
Commenters: aran
CC: aran, epriestley, tuomaspelkonen
Differential Revision: 325
Summary:
I'm probably missing some edge cases but it took me almost 3 hours to get this
far and I think it only makes things work that didn't work before. Some stuff
like SVN binary patches still won't work, although they should be far easier to
implement.
Most of the magic here just comes from reading the git source code. It appears
to work correctly; I sprinkled printf() around git liberally and recompiled it
during development. Took me about 45 minutes to figure out that "Index" vs
"index" causes git to silently fail in a confusing way. :/
Git has a diff mode for binary changes but I don't think we lose much by always
using the full binaries. We can enhance it later if we want.
Test Plan:
Exported and patched binary changes (a picture of a duck) into a working copy.
Reviewed By: aran
Reviewers: tuomaspelkonen, jungejason, aran
CC: simpkins, aran, epriestley
Differential Revision: 327
Summary:
passthru() doesn't show up on --trace, and one time a while ago someone had an
issue which was harder than necessary to debug because the command wasn't
avialable in the log. Use the logged version.
Also fix a locale issue I ran into on my local machine, with "en_US.utf8" not
being a valid locale.
Test Plan:
Created an SVN repo and used "arc commit" to make commits against it.
Reviewed By: jungejason
Reviewers: jungejason, tuomaspelkonen, aran
CC: aran, epriestley, jungejason
Differential Revision: 306
Summary:
The --only flag implies --nounit and --nolint, so there's really no
conflict if these flags are specified together.
(My wrapper around arc diff automatically specifies --nounit in some
cases. It is useful to be able to manually add --only in some cases,
and not have it fail because it conflicts with the automatically
specified --nounit flag.)
Test Plan:
Ran "arc diff --only --nounit".
Reviewed By: epriestley
Reviewers: epriestley, jungejason
CC: aran, epriestley, simpkins
Differential Revision: 298
Summary:
Update the lint workflow to exit immediately it there is no lint engine
configured. Previously it scanned the repository to find the paths to
check before failing in this case. Scanning the repository can be
relatively slow on large repositories.
Test Plan:
Ran "arc lint" in a large repository with no lint engine configured.
Verified that it failed quickly, without scanning the repository first.
Reviewed By: jungejason
Reviewers: jungejason, epriestley
CC: aran, jungejason
Differential Revision: 297
Summary:
The story for creating and maintaining libphutil libraries and modules
is pretty terrible right now: you need to know a bunch of secret scripts and
dark magic. Provide 'arc liberate' which endeavors to always do the right thing
and put a library in the correct state.
Test Plan:
Ran liberate on libphutil, arcanist, phabricator; created new
libphutil libraries, added classes to them, liberated everything, introduced
errors etc and liberated that stuff, nothing was obviously broken in a terrible
way..?
Reviewed By: aran
Reviewers: jungejason, tuomaspelkonen, aran
CC: aran, epriestley
Differential Revision: 269
Summary:
I only dropped this because it's slightly inconvient to accommodate, but
empirically it's pretty confusing to users (who often use --diff 12345 when they
mean --revision 12345).
Test Plan:
Ran "arc patch D45", "arc patch --revision 45", "arc help patch"
Reviewed By: tuomaspelkonen
Reviewers: tuomaspelkonen, jungejason, aran
CC: aran, tuomaspelkonen
Differential Revision: 241
Summary:
Provide a formal mechanism for making conduit calls from other scripts.
Test Plan:
Called 'conduit.ping'.
Reviewed By: jungejason
Reviewers: aran, jungejason, tuomaspelkonen
CC: aran, jungejason
Differential Revision: 239
Summary:
the current command to apply a git diff is 'git apply --index',
which will fail the whole patch and does not touch the working tree when
some of the hunks do not apply. We want to allow the user to patch the
ones which apply.
Test Plan:
run 'arc apply' against a diff whcih partially applies and
verify that the hunks which apply are applied, and .rej files are
generated for the rest.
Reviewed By: simpkins
Reviewers: simpkins, epriestley
Commenters: ju
CC: aran, slawekbiel, simpkins, ju
Differential Revision: 235
Summary:
Displaying each postponed test separately was spammy.
Now it will only display a single line with the total count
Test Plan:
~/local/arcanist/bin/arc unit --sandcastle-tests
35 tests to run.
Completed 'ParentInfoTestCase.php'
Interrupted 'MentionsUntagTestCase.php'
PASS ParentInfoTestCase
POSTPONED 34 tests
Reviewed By: sgrimm
Reviewers: jungejason, sgrimm
CC: sgrimm
Revert Plan:
sure
Other Notes:
Differential Revision: 173
Summary:
Refactor ArcanistLintRenderer into three independent classes, and let
the Workflow select between them based on its 'output' parameter.
Test Plan:
Introduce a Lint warning and lint with no --output, with --output summary and
with --output JSON.
Reviewed By: epriestley
Reviewers: epriestley
CC: aran, epriestley
Differential Revision: 96
Summary:
That's for the tests that will be run asynchronously.
Test Plan:
run arc unit with some tests that returned postponed status - they showed up.
Background was yellow.
Reviewed By: epriestley
CC: epriestley
Revert Plan:
sure
Other Notes:
Differential Revision: 77
Summary:
Test Plan:
$ arc patch --diff 553961
Patch deletes file 'html/miniprofile.php', but the file does not exist in
the working copy. Continue anyway? [y/N] y
Patch deletes file 'html/index.php', but the file does not exist in the
working copy. Continue anyway? [y/N] y
OKAY Successfully applied patch to the working copy.
Reviewers:
CC:
Summary: this was all kinds of fail when running from a subdirectory or on
invalid paths. Detect nonexistent paths; resolve valid paths.
Test Plan: ran "arc lint --lintall" on a file from a subdirectory, and on a
file which did not exist. Received patches and an error, respectively
Reviewers: arudolph
CC:
Differential Revision: 72
Summary:
When an arcbundle includes a symlink, we fail to apply it correctly
when applying to a subversion working copy.
Test Plan:
created a diff which added a symlink, removed it locally, bundled
and applied the patch, got a good symlink out
Reviewed By: aran
Reviewers: aran
CC: epriestley, aran
Differential Revision: 63
Summary:
If you apply an arcbundle to a subversion working copy which adds
files in new subdirectories, we fail to create and add the parent directories
so the whole operation fails. Make sure we create and add any missing parent
directories before apply patches.
Test Plan:
Applied Facebook diff #542056 to www@rE349795 cleanly, while it
failed previously.
Reviewed By: aran
Reviewers: aran
CC: aran
Differential Revision: 62
directories
Summary:
If you had changes to a directory covered by a .arcconfig residing
inside another directory also covered by an .arcconfig, the hook would
incorrectly attribute the first file it attempted to resolve to the topmost
.arcconfig because it failed to break out of the loop after resolving it.
All other files would be correctly attributed, because they'd hit the cache.
Test Plan:
Created an svn repository and checked it out locally. Created a
structure like this:
.arcconfig
shallow
a/
b/
c/
.arcconfig
d/
deep
deep2
deep3
Made modifications to "deep", "deep2", and "deep3". Received an error message
about multiple .arcconfig changes, attributing one file to the topmost
arcconfig and the other two to the deeper one.
Applied patch.
Commit now goes through. Made a commit affecting 'shallow' and 'deep' and
'deep2', commit was correctly blocked and files were attributed to the
corresponding .arcconfigs.
Reviewed By: mroch
Reviewers: mroch
CC: mroch
Differential Revision: 50
Summary: Haiping is getting a pretty confusing error message when trying to
commit.
Test Plan: Created a mock repository, installed the hook, made commits against
directories with bad .arcconfigs.
Reviewers:
CC:
Summary: When running "arc diff" in a mixed-base-revision working copy, we
prevent the operation. Relax this restriction so that having a different root
revision is okay, so long as all affected files share the same revision. This
facilitates multiple similar edits without updates.
Test Plan: Reverted a file to an older revision in an SVN working copy, ran
"arc diff", was rejected. Applied patch, ran "arc diff", diff went through and
was recorded with the right SVN base revision in the database.
Reviewers:
CC:
Summary: We sync from local right now, which is fairly terrible. But
fixing this correctly is also not trivial and I don't have the right
primitives yet.
Test Plan:
Reviewers:
CC:
Summary: The biggest blocker on getting rid of arc in trunk is that the lint
rules in the commit hooks are still running the old version. Push arc
commit hook support toward some reasonable state of approximately working.
Test Plan:
Reviewers:
CC:
Summary:
"-m ''" will still let you do a truly empty update if
you are insistent on that.
Test Plan:
meta
Differential Revision: 209194
Reviewed By: dschleimer
Reviewers: dschleimer
CC: epriestley
Revert Plan:
OK
Summary:
When you add new arguments to the unit command, they need to be
threaded through to the unit engine. This enables you to specify passthru
command which will be shipped through the callstack.
Test Plan:
Ran 'arc diff --maxtests 8 --apply-patches --trace' and verified
resulting behavior was correct.
Differential Revision: 207877
Reviewed By: dschleimer
Reviewers: dschleimer
CC: dschleimer
Revert Plan:
OK
Summary:
This should only happen on the 'diff' workflow.
Test Plan:
ran 'arc lint' and didn't get prompted to amend, ran 'arc diff' and got yelled
at for uncommitted changes, committed, ran 'arc diff' and got prompted to amend
Differential Revision: 206696
Reviewed By: adonohue
Reviewers: adonohue
CC: adonohue
Revert Plan:
OK