Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Stability bots spam #5200

Closed
annevk opened this issue Mar 22, 2017 · 9 comments
Closed

Stability bots spam #5200

annevk opened this issue Mar 22, 2017 · 9 comments
Assignees
Labels

Comments

@annevk
Copy link
Member

annevk commented Mar 22, 2017

See, e.g., #5195, for a recent example of too many comments by w3c-bots.

@jugglinmike
Copy link
Contributor

@bobholt I expect that this regression is due to w3c/prbuildbot#12, but only due to the timing between the merge and the incorrect behavior. I can't find any justification for this in the change set itself. I'm probably just being thick, though. I could use a second opinion; do you see what I'm missing?

The most likely cause is an incompatible change in the brittle relationship between the independent "title as identifier" functions, but they remain consistent:

So it's not clear to me why the prbuilbot no longer recognizes its own comments, as specified here: https://github.com/w3c/prbuildbot/blob/61f6626f992b85674e15ed600967346c5f7689db/github.py#L125-L131

@bobholt
Copy link
Contributor

bobholt commented Mar 22, 2017

@jugglinmike: Curious. I also don't see why that change would break that behavior, though I've seen it happening in another branch. I thought it was due to a change I made there, but it does coincide timing-wise with merging in your fix.

I'm having an issue getting my test server stood back up, but when it is, I'll take a peek at what's going on.

@bobholt
Copy link
Contributor

bobholt commented Mar 22, 2017

Running a test build. I've added logging to try to see what it's expecting and what it's really getting. May not get a final answer today, but I'll dive in again first thing in the morning.

@bobholt
Copy link
Contributor

bobholt commented Mar 22, 2017

So. This is unrelated to your change, @jugglinmike.

The prbuildbot is now looking for a title string matching # Ci_Stability.Sh Product=Chrome (unstable) #.

"That's odd," I said. It turns out the way this title string is originally determined is via partitioning environment variables received from Travis: https://github.com/w3c/prbuildbot/blob/master/travis.py#L68

And so this seemingly-innocuous change I noticed when rebasing my current working branch current master this afternoon seems to be the culprit: 8196377

It seems like that change by @gsnedders is true and good, and that the title partitioning logic in prbuildbot's travis.py needs to be revisited.

I can't look at it today, but I'll jump on it first thing in the morning if it's still outstanding.

@annevk
Copy link
Member Author

annevk commented Mar 23, 2017

See also #5208. wpt-pr-bot is also affected. Is that still under @tobie's control?

@tobie
Copy link
Contributor

tobie commented Mar 23, 2017

See also #5208. wpt-pr-bot is also affected. Is that still under @tobie's control?

Must be some API change. :-/

@tobie
Copy link
Contributor

tobie commented Mar 23, 2017

So for #5208 and wpt-pr-bot specifically, this looks like a rare racing condition: a new commit was pushed to the branch while the initial pull request was still being processed.

Guarding against this is something I systematically do nowadays but that I hadn't about back when I wrote wpt-pr-bot. It's probably not too difficult to fix, but I'd rather spend time elsewhere unless this becomes a recurrent problem.

@bobholt
Copy link
Contributor

bobholt commented Mar 23, 2017

I've opened a PR to resolve the new-comments-on-every-push behavior of the prbuildbot here.

@jugglinmike
Copy link
Contributor

Resolved via w3c/prbuildbot#13.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

5 participants