In Go 1.10 the output for tests was changed to have also a "(cached)" mode in
addition to the normal timing info printed. This is on by default. This adds
support for parsing these lines instead of erroring out on the regex.
Details
Details
- Reviewers
epriestley - Group Reviewers
Blessed Reviewers - Commits
- rARC7b6a9a93f4a4: (stable) Promote 2018 Week 10
rARCdcd7ef66d0e4: Add support for Go 1.10 (cached) output
Have a unit test included, and will continue to poke at it locally.
Diff Detail
Diff Detail
- Repository
- rARC Arcanist
- Lint
Lint Not Applicable - Unit
Tests Not Applicable
Event Timeline
| src/unit/parser/ArcanistGoTestResultParser.php | ||
|---|---|---|
| 71 | So close... Could make it its own variable if desired. | |
Comment Actions
FWIW, https://github.com/golang/go/issues/2981 provides JSON output of Go tests now, but that'd require much larger changes to adapt to.
Comment Actions
One minor quibble about line width inline.
See T10038 for broader guidance on lint/unit test bindings, but I'm okay with upstreaming this since is such a narrow fix.
| src/unit/parser/ArcanistGoTestResultParser.php | ||
|---|---|---|
| 71 | You should adhere to the 80 column rule. In this case, the easiest approach might be to remove the .* at the end, which is unnecessary: the pattern does only needs to match somewhere, it does not need to match the entire input. You could also replace the two instances of [\s\t] with \s, since \t is in the \s ("whitespace") class. I think the \w.* part of the pattern is also a little sketchy and will cause us to match the entire string first (inside the regex engine), then repeatedly backtrack to try to match the second delimiter. If possible (that is, if unit test names may never contain whitespace characters), it would be better to match \S+ ("not whitespace") instead of \w.* ('one word character followed by any number of any other character"), which saves one more character. The major goal being to simplify and improve the regexp, of course; the effect on the line width is just a fortunate side effect. (Is it possible for the 'name' to contain "(cached)" or values in the format "0.123s"?) In the more general case where you can't just shrink the regexp, breaking it up across multiple lines is usually the cleanest approach and can improve readability. | |
Comment Actions
The "Configure your editor to not use tabs" thing can be fixed by fiddling with .arclint, but obviously the file should contain a literal tab and you don't need to go fix linting for unit test data for this change.
(The approach we've taken in libphutil/ is to whitelist files in __tests__/data/, then move cases like this in there, but even if we take that approach here it should be in a separate change.)
Comment Actions
Shrunk the regex down by removing the big glob at the end and removing the spurious \ts like you mentioned. Not sure about whitespace in test files. But the unit tests are still passing with this method.
Comment Actions
I added you to Community, giving you sweeping janitorial powers on this install.
I added you to Blessed Committers, so you should be able to land this yourself. See that project's page for some help, or yell if you run into issues.
Comment Actions
Very confused here -
Set my origin remote to ssh://dweller@secure.phabricator.com/diffusion/P/arcanist.git like instructed in the community docs, and verified everything worked via SSH.
But master (42e5b8a04bece0abe4e018deb381347ac1bf7d37) seems to not contain any such files as src/unit/parser/ArcanistGoTestResultParser.php.
arc unit prints out a mysterious failure now that I've changed my origin remote from Github to the mentioned one:
Exception Command failed with error #1! COMMAND git merge-base 'origin/master' HEAD STDOUT (empty) STDERR (empty) (Run with `--trace` for a full exception trace.)
Attempting to rebase my branch on top of origin/master is a bit of a mess.
I originally started working from the Github master branch, which I'm assuming is the source of these issues? Are the parsers in an different repo in the upstream or something like that?
What should I do?
Comment Actions
In this URI:
ssh://dweller@secure.phabricator.com/diffusion/P/arcanist.git
...this is the actual meaning:
ssh://dweller@secure.phabricator.com/diffusion/P/arcanist.git
^ ^^^^^^^^^^^^
| |
| +- Human readable, arbitrary text which is ignored by the server.
|
+-------------- Callsign which identifies which repository you want to clone.That is, changing phabricator.git to arcanist.git without changing P to ARC means that you're still cloning Phabricator, you're just calling it arcanist.git.
The right URI for the Arcanist upstream can be found on the Arcanist repository page:
...and is:
ssh://secure@secure.phabricator.com/diffusion/ARC/arcanist.git
Try using that URI (with ARC, not P!) instead?
Comment Actions
(The project description is a little out of date and could be more clear about this.)