Page MenuHomePhabricator

treemanifest: make tests more lenient and fail more descriptively
ClosedPublic

Authored by ryanmce on Aug 16 2017, 8:39 AM.

Details

Summary

The test-treemanifest-pushrebase test was failing in our continuous integration
environment with a perplexing error:

$ pushclients
+ abort: exporting bookmark master failed!

(plus some additional failures later)

I thought this should not be possible because of the redirect, but it turns out
that the order of the redirects matters:

>/dev/null 2>&1 redirects both stderr and stdout to /dev/null, but
2>&1 >/dev/null redirects stdout to /dev/null and stderr to stdout

(TIL)

However, this was the only clue to the issue that was going on, and so it's
better to actually keep the error around, if one occurs, so I removed the 2>&1
redirect altogether.

Furthermore, the test was failing with various clients failing to push.
To increase the chance of this succeeding, I'm bumping the sleep time. Ideally,
we should wait for the server to be "ready", but I'm not certain what that means
or how to test and would like to discuss to figure it out. This hopefully will
unblock the continuous integration for now.

Test Plan

Test still passes locally; wait to see if it passes in automation.

Diff Detail

Repository
rFBHGX Facebook Mercurial Extensions
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

ryanmce created this revision.Aug 16 2017, 8:39 AM
ryanmce retitled this revision from [treemanifest] make tests more lenient and fail more descriptively to treemanifest: make tests more lenient and fail more descriptively.Aug 16 2017, 11:05 AM
mitrandir accepted this revision.Aug 16 2017, 11:05 AM
This revision is now accepted and ready to land.Aug 16 2017, 11:05 AM
This revision was automatically updated to reflect the committed changes.