Page MenuHomePhabricator

wmf_style check in puppet silently fails when it finds the addition of an error that was also already occurring in the same file
Open, MediumPublicBUG REPORT

Description

https://gerrit.wikimedia.org/r/#/c/operations/puppet/+/498641/ PS3
One weird thing in the result is the wmf_style checker saying delta 1 but showing no new violations. It's probably that.
The tox logs thing is probably a red herring due to || true.

Event Timeline

Krenair created this task.Mar 24 2019, 2:34 AM
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptMar 24 2019, 2:34 AM
Krenair renamed this task from Unexplained failure of puppet commit to Unexplained failure of puppet commit, possibly wmf_style related.Mar 24 2019, 2:40 AM
Krenair added a comment.EditedMar 24 2019, 8:01 PM

So I read through the code that does this in rake_modules/taskgen.rb and got suspicious that something appearing in old_problems once but new_problems twice with two different lines would set delta = 1, cause an abort, but not get shown due to already having a match according to the other.select { |x| x[:message] == p[:message] && x[:path] == p[:path] }.empty? line in print_wmf_style_violations.
I tried https://gerrit.wikimedia.org/r/#/c/operations/puppet/+/498641/4/rake_modules/taskgen.rb and it proved this: https://integration.wikimedia.org/ci/job/operations-puppet-tests-stretch-docker/8606/console

19:58:31 DEBUG: NEW PROBLEMS:
19:58:31 modules/uwsgi/manifests/app.pp:54 wmf-style: 'uwsgi::app' should not include the deprecated define 'base::service_unit'
19:58:31 modules/uwsgi/manifests/app.pp:71 wmf-style: 'uwsgi::app' should not include the deprecated define 'base::service_unit'
19:58:31 DEBUG: OLD PROBLEMS:
19:58:31 modules/uwsgi/manifests/app.pp:54 wmf-style: 'uwsgi::app' should not include the deprecated define 'base::service_unit'
19:58:31 NEW violations:
19:58:31 Nothing found
19:58:31 Resolved violations:
19:58:31 Nothing found
Krenair renamed this task from Unexplained failure of puppet commit, possibly wmf_style related to wmf_style check in puppet silently fails when it finds the addition of an error that was also already occurring in the same file.Mar 24 2019, 8:01 PM

I made https://gerrit.wikimedia.org/r/#/c/operations/puppet/+/498641/5/rake_modules/taskgen.rb and that shows the error: https://integration.wikimedia.org/ci/job/operations-puppet-tests-stretch-docker/8607/console

20:05:26 ---> wmf_style lint
20:05:26 wmf-style: total violations delta 1
20:05:26 NEW violations:
20:05:26 modules/uwsgi/manifests/app.pp:71 wmf-style: 'uwsgi::app' should not include the deprecated define 'base::service_unit'
20:05:26 Resolved violations:
20:05:26 Nothing found
20:05:26 ---> end wmf_style lint
Krenair claimed this task.Mar 24 2019, 8:10 PM

Change 498731 had a related patch set uploaded (by Alex Monk; owner: Alex Monk):
[operations/puppet@production] wmf_style lint: Don't silently fail on new errors that already appear on other lines

https://gerrit.wikimedia.org/r/498731

Change 498731 abandoned by Alex Monk:
[operations/puppet@production] wmf_style lint: Show new errors that already appear on other lines

Reason:

https://gerrit.wikimedia.org/r/498731

Krenair removed Krenair as the assignee of this task.Sun, Nov 22, 4:41 AM
Dzahn triaged this task as Medium priority.Mon, Nov 23, 11:04 PM