Page MenuHomePhabricator

mcorteel (Maxime Corteel)
User

Projects

User does not belong to any projects.

Today

  • Clear sailing ahead.

Tomorrow

  • Clear sailing ahead.

Wednesday

  • Clear sailing ahead.

User Details

User Since
Aug 2 2016, 11:44 AM (123 w, 6 d)
Availability
Available

Recent Activity

Oct 23 2018

mcorteel added a comment to T12785: Arc unit command fails with PHPUnit 6.

Well, this sucks... But thank you for taking the time to explain the situation ๐Ÿ˜ƒ

Oct 23 2018, 2:17 PM ยท Arcanist, Bug Report
mcorteel added a comment to T12785: Arc unit command fails with PHPUnit 6.

@epriestley Does this mean that phabricator isn't open to contributions?

Oct 23 2018, 12:28 PM ยท Arcanist, Bug Report

Jul 2 2018

mcorteel added a comment to P2084 T12785 patch.

I didn't even suggest dropping support, I'm merely listing the options to allow both versions to work.
We use a manually patched version of arcanist because it suits our company's use case, but it wouldn't be suitable for a merge in Phabricator's codebase in this state.

Jul 2 2018, 8:39 AM

Jun 29 2018

mcorteel added a comment to P2084 T12785 patch.

@DragonBe Backwards compatibility is broken by this fix. I'm not sure what the phabricator policy is regarding this, but I see two options:

  • Creating another test engine (one for PHPUnit <= 5 and one for PHPUnit > 5)
  • Parsing output differently depending on the PHPUnit version and reuse the old code where needed. This would require a little work to do things properly I guess.
Jun 29 2018, 10:54 AM

Jun 4 2018

mcorteel awarded T4280: Embed anchor in remark up text and later link to it. a Love token.
Jun 4 2018, 1:41 PM ยท Remarkup

Feb 21 2018

mcorteel added a comment to P2084 T12785 patch.

Once I get an answer on how to submit it... See T12785

Feb 21 2018, 4:18 PM

Feb 16 2018

mcorteel added a comment to T8824: Allow relative URLs in custom fields of type "link".

It would be perfect if the syntax supported in remarkup links could be used, which means that for @swisspol's example, projects/phabricator_transition/ would translate to https://temp.phacility.com/w/projects/phabricator_transition/.
And it would be even more awesome if the behavior of the displayed link in the "view" page could be the same, which means that in this case, it wouldn't display the link but the name of the Phriction page!

Feb 16 2018, 2:47 PM ยท Custom Fields
mcorteel awarded T8824: Allow relative URLs in custom fields of type "link" a Like token.
Feb 16 2018, 1:56 PM ยท Custom Fields

Feb 1 2018

mcorteel added a comment to T12785: Arc unit command fails with PHPUnit 6.

@cmmata here is my diff. It can probably be improved, but it works for me:

1diff --git a/src/unit/engine/PhpunitTestEngine.php b/src/unit/engine/PhpunitTestEngine.php
2index 8206b787..ef01fda4 100644
3--- a/src/unit/engine/PhpunitTestEngine.php
4+++ b/src/unit/engine/PhpunitTestEngine.php
5@@ -52,7 +52,7 @@ final class PhpunitTestEngine extends ArcanistUnitTestEngine {
6 if (!Filesystem::pathExists($test_path)) {
7 continue;
8 }
9- $json_tmp = new TempFile();
10+ $xml_tmp = new TempFile();
11 $clover_tmp = null;
12 $clover = null;
13 if ($this->getEnableCoverage() !== false) {
14@@ -64,10 +64,10 @@ final class PhpunitTestEngine extends ArcanistUnitTestEngine {
15
16 $stderr = '-d display_errors=stderr';
17
18- $futures[$test_path] = new ExecFuture('%C %C %C --log-json %s %C %s',
19- $this->phpunitBinary, $config, $stderr, $json_tmp, $clover, $test_path);
20+ $futures[$test_path] = new ExecFuture('%C %C %C --log-junit %s %C %s',
21+ $this->phpunitBinary, $config, $stderr, $xml_tmp, $clover, $test_path);
22 $tmpfiles[$test_path] = array(
23- 'json' => $json_tmp,
24+ 'xml' => $xml_tmp,
25 'clover' => $clover_tmp,
26 );
27 }
28@@ -81,7 +81,7 @@ final class PhpunitTestEngine extends ArcanistUnitTestEngine {
29
30 $results[] = $this->parseTestResults(
31 $test,
32- $tmpfiles[$test]['json'],
33+ $tmpfiles[$test]['xml'],
34 $tmpfiles[$test]['clover'],
35 $stderr);
36 }
37@@ -90,17 +90,17 @@ final class PhpunitTestEngine extends ArcanistUnitTestEngine {
38 }
39
40 /**
41- * Parse test results from phpunit json report.
42+ * Parse test results from phpunit XML report.
43 *
44 * @param string $path Path to test
45- * @param string $json_tmp Path to phpunit json report
46+ * @param string $xml_tmp Path to phpunit XML report
47 * @param string $clover_tmp Path to phpunit clover report
48 * @param string $stderr Data written to stderr
49 *
50 * @return array
51 */
52- private function parseTestResults($path, $json_tmp, $clover_tmp, $stderr) {
53- $test_results = Filesystem::readFile($json_tmp);
54+ private function parseTestResults($path, $xml_tmp, $clover_tmp, $stderr) {
55+ $test_results = Filesystem::readFile($xml_tmp);
56 return id(new ArcanistPhpunitTestResultParser())
57 ->setEnableCoverage($this->getEnableCoverage())
58 ->setProjectRoot($this->projectRoot)
59diff --git a/src/unit/parser/ArcanistPhpunitTestResultParser.php b/src/unit/parser/ArcanistPhpunitTestResultParser.php
60index 5ccff970..3d7fcd77 100644
61--- a/src/unit/parser/ArcanistPhpunitTestResultParser.php
62+++ b/src/unit/parser/ArcanistPhpunitTestResultParser.php
63@@ -9,10 +9,10 @@
64 final class ArcanistPhpunitTestResultParser extends ArcanistTestResultParser {
65
66 /**
67- * Parse test results from phpunit json report
68+ * Parse test results from phpunit XML report
69 *
70 * @param string $path Path to test
71- * @param string $test_results String containing phpunit json report
72+ * @param string $test_results String containing phpunit XML report
73 *
74 * @return array
75 */
76@@ -25,7 +25,7 @@ final class ArcanistPhpunitTestResultParser extends ArcanistTestResultParser {
77 return array($result);
78 }
79
80- $report = $this->getJsonReport($test_results);
81+ $report = simplexml_load_string($test_results);
82
83 // coverage is for all testcases in the executed $path
84 $coverage = array();
85@@ -36,56 +36,36 @@ final class ArcanistPhpunitTestResultParser extends ArcanistTestResultParser {
86 $last_test_finished = true;
87
88 $results = array();
89- foreach ($report as $event) {
90- switch (idx($event, 'event')) {
91- case 'test':
92- break;
93- case 'testStart':
94- $last_test_finished = false;
95- // fall through
96- default:
97- continue 2; // switch + loop
98- }
99-
100+ foreach ($report->testsuite as $test_suite) {
101 $status = ArcanistUnitTestResult::RESULT_PASS;
102 $user_data = '';
103
104- if ('fail' == idx($event, 'status')) {
105+ if ((int)$test_suite['failures'] > 0) {
106 $status = ArcanistUnitTestResult::RESULT_FAIL;
107- $user_data .= idx($event, 'message')."\n";
108- foreach (idx($event, 'trace') as $trace) {
109- $user_data .= sprintf(
110- "\n%s:%s",
111- idx($trace, 'file'),
112- idx($trace, 'line'));
113+ foreach ($test_suite->testcase as $test_case) {
114+ foreach ($test_case->failure as $failure) {
115+ $user_data .= sprintf(
116+ "\n%s",
117+ (string)$failure);
118+ }
119 }
120- } else if ('error' == idx($event, 'status')) {
121- if (strpos(idx($event, 'message'), 'Skipped Test') !== false) {
122- $status = ArcanistUnitTestResult::RESULT_SKIP;
123- $user_data .= idx($event, 'message');
124- } else if (strpos(
125- idx($event, 'message'),
126- 'Incomplete Test') !== false) {
127- $status = ArcanistUnitTestResult::RESULT_SKIP;
128- $user_data .= idx($event, 'message');
129- } else {
130- $status = ArcanistUnitTestResult::RESULT_BROKEN;
131- $user_data .= idx($event, 'message');
132- foreach (idx($event, 'trace') as $trace) {
133- $user_data .= sprintf(
134- "\n%s:%s",
135- idx($trace, 'file'),
136- idx($trace, 'line'));
137+ } else if ($test_suite['errors'] > 0) {
138+ $status = ArcanistUnitTestResult::RESULT_BROKEN;
139+ foreach ($test_suite->testcase as $test_case) {
140+ foreach ($test_case->error as $error) {
141+ $user_data .= sprintf(
142+ "\n%s",
143+ (string)$error);
144 }
145 }
146 }
147
148- $name = preg_replace('/ \(.*\)/s', '', idx($event, 'test'));
149+ $name = preg_replace('/ \(.*\)/s', '', $test_suite['name']);
150
151 $result = new ArcanistUnitTestResult();
152 $result->setName($name);
153 $result->setResult($status);
154- $result->setDuration(idx($event, 'time'));
155+ $result->setDuration((float)$test_suite['time']);
156 $result->setCoverage($coverage);
157 $result->setUserData($user_data);
158
159@@ -95,7 +75,7 @@ final class ArcanistPhpunitTestResultParser extends ArcanistTestResultParser {
160
161 if (!$last_test_finished) {
162 $results[] = id(new ArcanistUnitTestResult())
163- ->setName(idx($event, 'test')) // use last event
164+ ->setName($test_suite['name']) // use last event
165 ->setUserData($this->stderr)
166 ->setResult(ArcanistUnitTestResult::RESULT_BROKEN);
167 }
168@@ -161,28 +141,4 @@ final class ArcanistPhpunitTestResultParser extends ArcanistTestResultParser {
169 return $reports;
170 }
171
172- /**
173- * We need this non-sense to make json generated by phpunit
174- * valid.
175- *
176- * @param string $json String containing JSON report
177- * @return array JSON decoded array
178- */
179- private function getJsonReport($json) {
180-
181- if (empty($json)) {
182- throw new Exception(
183- pht(
184- 'JSON report file is empty, it probably means that phpunit '.
185- 'failed to run tests. Try running %s with %s option and then run '.
186- 'generated phpunit command yourself, you might get the answer.',
187- 'arc unit',
188- '--trace'));
189- }
190-
191- $json = preg_replace('/}{\s*"/', '},{"', $json);
192- $json = '['.$json.']';
193- return phutil_json_decode($json);
194- }
195-
196 }

Feb 1 2018, 11:00 AM ยท Arcanist, Bug Report
mcorteel created P2084 T12785 patch.
Feb 1 2018, 10:58 AM

Nov 21 2017

mcorteel updated subscribers of T12785: Arc unit command fails with PHPUnit 6.

@epriestley I tried submitting a patch, but I get a Permission denied (publickey) which results in Unable to push changes to the staging area..
I added my public key and created a config rule to use it for requests to secure.phabricator.com, I'm not sure what else to do.

Nov 21 2017, 1:37 PM ยท Arcanist, Bug Report
mcorteel added a comment to T12785: Arc unit command fails with PHPUnit 6.

So, to recap:

  • PhpunitTestEngine::run must be updated to use --log-junit which will output XML instead of JSON
  • ArcanistPhpunitTestResultParser::parseTestResults must be updated to parse this XML file.

This seems simple enough, I will try to make it work.

Nov 21 2017, 11:28 AM ยท Arcanist, Bug Report

Oct 16 2017

mcorteel awarded T7332: Notification / Message icons should update in real-time a Love token.
Oct 16 2017, 1:56 PM ยท Conpherence (v4), Aphlict, Notifications

Aug 22 2017

mcorteel edited the content of Organizations Using Phabricator.
Aug 22 2017, 11:47 AM

Jun 23 2017

mcorteel added a comment to T12865: Strikethrough for icons in checked remarkup list items.

Sorry, I should have led with a screenshot! I'm the biggest nitpicker, I know.

Jun 23 2017, 5:48 PM ยท Remarkup, Feature Request
mcorteel added a comment to T12865: Strikethrough for icons in checked remarkup list items.

No they're not. I know it's a tiny difference, but look at this closely (left is my proposal, right is the current state):

Jun 23 2017, 4:41 PM ยท Remarkup, Feature Request

Jun 22 2017

mcorteel created T12865: Strikethrough for icons in checked remarkup list items.
Jun 22 2017, 8:24 AM ยท Remarkup, Feature Request

May 19 2017

mcorteel added a comment to T12728: Newly created Phriction document doesn't appear in hierarchy.

And by doing that, I realize that I have been keeping up with master instead of stable like an (oh, there's no donkey emoji, too bad...).
So it's fixed... Sorry!

May 19 2017, 1:59 PM ยท Phriction, Bug Report
mcorteel added a comment to T12728: Newly created Phriction document doesn't appear in hierarchy.

Sorry, it completely slipped my mind!

May 19 2017, 1:54 PM ยท Phriction, Bug Report
mcorteel added a comment to T12728: Newly created Phriction document doesn't appear in hierarchy.

I have been updating the DB manually to set status back to 0 as a workaround for now. But you could probably see how this affects our workflow!

May 19 2017, 9:19 AM ยท Phriction, Bug Report
mcorteel created T12728: Newly created Phriction document doesn't appear in hierarchy.
May 19 2017, 9:17 AM ยท Phriction, Bug Report
mcorteel awarded T3967: Build a 2-up preview mode into the fullscreen Remarkup editor a Like token.
May 19 2017, 8:56 AM ยท Remarkup

Apr 25 2017

mcorteel awarded T1562: Build "Facts", an ETL pipeline and charting application a Love token.
Apr 25 2017, 2:01 PM ยท Restricted Project, Wikimedia, Facts
mcorteel added a comment to T12640: Project descriptions remarkup is not parsed in query object selection.

OK. But isn't it weird from an end-user POV to have unparsed remarkup there?
If it's a choice, you can close this issue! Thank you for your quick answer.

Apr 25 2017, 1:55 PM ยท Bug Report
mcorteel created T12641: Maniphest burnup rate report: by user.
Apr 25 2017, 12:10 PM ยท Maniphest, Feature Request
mcorteel created T12640: Project descriptions remarkup is not parsed in query object selection.
Apr 25 2017, 12:06 PM ยท Bug Report

Mar 22 2017

mcorteel added a comment to T12357: Remarkup: **bold** effect in Header section is not discernible.

We tested this style (black with shadow) on our install, but the consensus is that it looks awful...
I guess a simple black text color is a little subtle, but at least it isn't confusing (like the blue color) or ugly (like the shadow).
@chad what do you think?

Mar 22 2017, 6:22 AM ยท Remarkup, Bug Report

Mar 8 2017

mcorteel added a comment to T12284: Wrong notification label (subtask status update).

Thank you for the very thorough reply.
That's not a big deal and it certainly can wait!

Mar 8 2017, 12:46 PM ยท Bug Report
mcorteel added a comment to T12357: Remarkup: **bold** effect in Header section is not discernible.

Thank you for your quick reaction (and sorry that I didn't reply sooner). I think a black color with a 1px black text-shadow looks better. The blue is confusing (it sort of looks like a link).
So I suggest:

.phui-document-view .phabricator-remarkup .remarkup-header strong {
  color: #000000;
  text-shadow: 0 0 1px #000000;
}
Mar 8 2017, 12:42 PM ยท Remarkup, Bug Report

Mar 6 2017

mcorteel created T12357: Remarkup: **bold** effect in Header section is not discernible.
Mar 6 2017, 10:23 AM ยท Remarkup, Bug Report

Feb 17 2017

mcorteel created T12284: Wrong notification label (subtask status update).
Feb 17 2017, 10:55 AM ยท Bug Report

Feb 7 2017

mcorteel added a comment to T12213: Task title is truncated in task graph, even if there is enough space.

Thank you for your quick reply. Indeed, this is tricky. Apparently, max-width has to be set on cells for overflow to work.

Feb 7 2017, 6:31 AM ยท Maniphest, Bug Report

Feb 6 2017

mcorteel awarded T5427: Force a line break in a table cell a Like token.
Feb 6 2017, 4:03 PM ยท Remarkup
mcorteel created T12213: Task title is truncated in task graph, even if there is enough space.
Feb 6 2017, 9:37 AM ยท Maniphest, Bug Report

Aug 2 2016

mcorteel created T11414: Clicking on mock inline comment preview doesn't display the correct image.
Aug 2 2016, 11:55 AM ยท Restricted Project, Pholio, Bug Report