Closed Bug 1217912 Opened 9 years ago Closed 7 years ago

Prevent running Buildbot jobs without --config-file set

Categories

(Release Engineering :: General, defect)

defect
Not set
normal

Tracking

(Not tracked)

RESOLVED INCOMPLETE

People

(Reporter: armenzg, Unassigned)

Details

Attachments

(2 files)

Issues like this should be prevented to help the author, sheriffs and unicorns.
https://bugzilla.mozilla.org/show_bug.cgi?id=1211889#c20

I will take it.
Attached patch fix.custom.diffSplinter Review
Attachment #8679666 - Flags: review?(rail)
Attached patch fix.configs.diffSplinter Review
The bleeding shall end!

The output below is with jmaher's patch.
(venv)armenzg@armenzg-thinkpad:~/.mozilla/releng/repos/buildbot-configs/master$ buildbot checkconfig .
This builder does not have a --cfg file (add 'no_config_file' if it applies) Ubuntu ASAN VM 12.04 x64 b2g-inbound opt test mochitest-chrome-1
Traceback (most recent call last):
  File "/home/armenzg/.mozilla/releng/repos/buildbot/master/buildbot/scripts/runner.py", line 1040, in doCheckConfig
    ConfigLoader(basedir=configFileName)
  File "/home/armenzg/.mozilla/releng/repos/buildbot/master/buildbot/scripts/checkconfig.py", line 31, in __init__
    self.loadConfig(configFile, check_synchronously_only=True)
  File "/home/armenzg/.mozilla/releng/repos/buildbot/master/buildbot/master.py", line 652, in loadConfig
    exec f in localDict
  File "./master.cfg", line 156, in <module>
    BRANCH_UNITTEST_VARS['platforms'])
  File "/home/armenzg/.mozilla/releng/repos/buildbotcustom/misc.py", line 2654, in generateTalosBranchObjects
    **test_builder_kwargs
  File "/home/armenzg/.mozilla/releng/repos/buildbotcustom/misc.py", line 934, in generateChunkedUnittestBuilders
    *args, **kwargs_copy
  File "/home/armenzg/.mozilla/releng/repos/buildbotcustom/misc.py", line 898, in generateUnittestBuilders
    builders.extend(generateTestBuilder(**test_builder_kwargs))
  File "/home/armenzg/.mozilla/releng/repos/buildbotcustom/misc.py", line 784, in generateTestBuilder
    sys.exit(1)
SystemExit: 1
Attachment #8679667 - Flags: review?(rail)
Comment on attachment 8679666 [details] [diff] [review]
fix.custom.diff

I'm not comfortable adding sys.exit(1) into buildbot code. This may bring down the whole farm down because someone forgot to add "--cfg".

I'd rather add some unittests to catch this kind of errors.

Also, "-c" and "--config-file" are legit equivalents of "--cfg".
Attachment #8679666 - Flags: review?(rail) → review-
Attachment #8679667 - Flags: review?(rail)
I don't know how to write such a unit test.
Assignee: armenzg → nobody
Would using an assert be better? If the builder requires it, I think it's fair to enforce it in the code.
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → INCOMPLETE
Component: General Automation → General
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: