On Wed, Jun 22, 2016 at 7:38 PM, Boris Zbarsky <bzbar...@mit.edu> wrote:
> As a matter of dealing with test breakage and how it's resolved, do you
> also prefer doing a bunch of commits and only then running tests, and if
> they fail closing down the tree until it's all resolved? If not, how is
> this substantively different? I realize it's different in a practical
> sense because the costs are higher than normal per-commit testing, of
> course.
>
>
We don't close down the tree except for CI fires (and in the past, large
bitrot-prone rustups). We currently have tests run on every approved PR
(one at a time) before they are merged. We like for individual commits to
pass tests for bisecting to work well but it is not a hard requirement. If
the tests fail for a PR, it is taken out of the queue until it is fixed and
reapproved. In the meantime, the next PR in the queue is tested. We also
have travis perform a reduced set of tests on PRs so you can catch some
things that may cause it to fail before approving. Additionally, we have
the ability to "try" a PR so that it will be tested against the full suite
but not merged. We never have backouts, because master always is green.
m-c does not have this model, and I'd also like to prevent marrying our CI
times to those of m-c, or being affected by m-c backouts. That's why I like
the sync model, it concentrates all of the conflict in one operation,
instead of distributing it everywhere and making every PR that touches
anything that affects style suffer. The syncing operation is also async
(ha!) -- while being performed other Servo work can continue. Sending PRs
which look like they need testing to gecko-try should catch most issues,
the only remaining issues are when a PR (like a wrong refactor, or
something) which wasn't sent through gecko-try breaks stylo, or when
something on Gecko's vendor dir managed to land between the time the Servo
PR was tested and landed. I postulate these two cases will be relatively
rare, and these will be the only times the sync fails.
In most cases, I envision the workflow on the servo side being like:
1. Reviewer sees PR that may need stylo testing
2. Reviewer goes through the review process
3. When the process is nearly over (just nits left, or whatever)
reviewer sends PR to gecko-try via a bors command (also perhaps servo-try)
4. gecko-try says:
1. OK:
1. Reviewer approves PR
2. Our autolander queues the PR up for testing and eventually
merges it if it passes. If it fails, fix it. Goto 3 if necessary, else
reapprove.
3. Perhaps trigger a sync to avoid future conflicts. This can be
fully automated.
2. Failed:
1. Bad luck, one of the "relatively rare" things happened. Perform
a sync locally, manually fixing the errors.
2. Rebase PR and goto 3
The gecko side keeps its current workflow, with the exception that:
- PRs which may affect servo tick a servo-stylo option in the trychooser
before their try run
- After such PRs merge, attempt to trigger an automated sync. Not
necessary, as before, depends on the situation.
This is actually pretty close to the proposed model in the doc, but it
gives a lot more leeway in when the sync should happen. The proposed model
is basically equivalent to this except syncs are mandatorily part of any PR
that affects style, and can make the queue clog up. Here based on various
factors you can choose not to sync (e.g. if the queue is already full; you
can wait to trigger the sync until it empties, or if the PR probably
doesn't need a sync). We can then by trial and error figure out the best
policy for when to sync and when not to sync balancing the need for fast CI
times and the need for things to not cause colossal merge conflicts.
> You make it sound like refactorings don't introduce bugs. ;)
>
Sure they can, but they should be less likely to :)
We will still have the full CI run on a sync so these will get caught, just
not immediately.
-Manish Goregaokar
_______________________________________________
dev-servo mailing list
dev-servo@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-servo
