Differential D328

resultsdb_directive raises an exception when check_detail is missing data
ClosedPublic
Actions

Authored by jskladan on Apr 7 2015, 1:51 PM.

Details

Reviewers
kparal
tflink
mkrizek
Maniphest Tasks
T447: resultsdb_directive should raise an exception when check_detail is missing data
Commits
rLTRNbb86d0ff4ebd: resultsdb_directive raises an exception when check_detail is missing data
Summary

resultsdb_directive now checks the CheckDetails produced from the TAP given on input
it the CheckDetails miss either item or report_type (meaning, that the values are set
to 'non-True-like'), proper exception is raised

Test Plan

unittests work

Diff Detail

Repository
rLTRN libtaskotron
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
jskladan retitled this revision from to resultsdb_directive raises an exception when check_detail is missing data.Apr 7 2015, 1:51 PM
jskladan updated this object.
jskladan edited the test plan for this revision. (Show Details)
jskladan added reviewers: kparal, tflink, mkrizek.
kparal accepted this revision.Apr 7 2015, 2:26 PM

LGTM, but please adjust the documentation before pushing.

libtaskotron/directives/resultsdb_directive.py
33–34

This should be amended to also mention that we raise this if certain yaml keys (let's name them) are missing.

163

I guess we should change this to

TAP output parses OK

Otherwise we claim that TAP is OK on one line, and then immediately fail on the next line saying that e.g. 'item' is missing.

testing/test_resultsdb_directive.py
101–105

Alternatively,

tap = '\n'.join([line for line in self.ref_input['results'].splitlines() if 'item:' not in line])

However, not sure if that's more readable :)

This revision is now accepted and ready to land.Apr 7 2015, 2:26 PM
jskladan updated this revision to Diff 895.Apr 7 2015, 2:34 PM
  • Fixed according to review
mkrizek accepted this revision.Apr 8 2015, 6:09 AM
Closed by commit rLTRNbb86d0ff4ebd: resultsdb_directive raises an exception when check_detail is missing data (authored by Josef Skladanka <jskladan@redhat.com>). · Explain WhyApr 8 2015, 1:08 PM
This revision was automatically updated to reflect the committed changes.

Revision Contents

Diff 907

View Options

libtaskotron/directives/resultsdb_directive.py

Show All 24 Lines22 results:
25 type: str 25​ type: str
26returns: | 26​returns: |
27 A list of HTTP response objects, one for every HTTP request sent to create a 27​ A list of HTTP response objects, one for every HTTP request sent to create a
28 result in ResultsDB (i.e. there should be the same number of responses as 28​ result in ResultsDB (i.e. there should be the same number of responses as
29 there are results in ``results`` parameter). The response objects are created 29​ there are results in ``results`` parameter). The response objects are created
30 by :meth:`requests.Response.json`, i.e. they are python dictionaries created 30​ by :meth:`requests.Response.json`, i.e. they are python dictionaries created
31 from ResultsDB's response JSON object. 31​ from ResultsDB's response JSON object.
32raises: | 32​raises: |
33 * :class:`.TaskotronDirectiveError`: if ``results`` is not in a valid TAP13 33​ * :class:`.TaskotronDirectiveError`: if ``results`` is not in a valid TAP13
34 format 34​ format, or when ``item`` or ``type`` are missing in the TAP's result data
kparalUnsubmitted
Not Done

This should be amended to also mention that we raise this if certain yaml keys (let's name them) are missing.

35 * :class:`resultsdb_api.ResultsDBapiException`: for any errors coming from 35​ * :class:`resultsdb_api.ResultsDBapiException`: for any errors coming from
36 the ResultsDB server 36​ the ResultsDB server
37version_added: 0.4 37​version_added: 0.4
38""" 38​"""
39 39​
40EXAMPLES = """ 40​EXAMPLES = """
41These two actions first run ``run_rpmlint.py`` and export its TAP13 output to 41​These two actions first run ``run_rpmlint.py`` and export its TAP13 output to
42``${rpmlint_output}``, and then feed this TAP13 output to ``resultsdb`` 42​``${rpmlint_output}``, and then feed this TAP13 output to ``resultsdb``
▲ Show 20 Lines • Show All 112 Lines • ▼ Show 20 Line(s)151 def process(self, input_data, env_data):
155 if 'checkname' not in env_data: 155​ if 'checkname' not in env_data:
156 raise TaskotronDirectiveError("The resultsdb directive requires "\ 156​ raise TaskotronDirectiveError("The resultsdb directive requires "\
157 "resultsdb_job_id and checkname.") 157​ "resultsdb_job_id and checkname.")
158 158​
159 # checking if reporting is enabled is done after importing tap which 159​ # checking if reporting is enabled is done after importing tap which
160 # serves as validation of input results 160​ # serves as validation of input results
161 try: 161​ try:
162 check_details = check.import_TAP(input_data['results']) 162​ check_details = check.import_TAP(input_data['results'])
163 log.debug("TAP output is OK.") 163​ log.debug("TAP output parsed OK.")
kparalUnsubmitted
Not Done

I guess we should change this to

TAP output parses OK

Otherwise we claim that TAP is OK on one line, and then immediately fail on the next line saying that e.g. 'item' is missing.

164 except TaskotronValueError as e: 164​ except TaskotronValueError as e:
165 raise TaskotronDirectiveError("Failed to load 'results': %s" % e.message) 165​ raise TaskotronDirectiveError("Failed to load 'results': %s" % e.message)
166 166​
167​ for detail in check_details:
168​ if not (detail.item and detail.report_type):
169​ raise TaskotronDirectiveError("The resultsdb directive requires "
170​ "'item' and 'type' to be present in the TAP data.")
171​
167 conf = config.get_config() 172​ conf = config.get_config()
168 if not (conf.reporting_enabled and conf.report_to_resultsdb): 173​ if not (conf.reporting_enabled and conf.report_to_resultsdb):
169 log.info("Reporting to ResultsDB is disabled. Once enabled, the " 174​ log.info("Reporting to ResultsDB is disabled. Once enabled, the "
170 "following would get reported:\n%s" % 175​ "following would get reported:\n%s" %
171 report_summary(input_data['results'], env_data['checkname'])) 176​ report_summary(input_data['results'], env_data['checkname']))
172 log.info('Hint: Enabling debug output allows you to see unstripped ' 177​ log.info('Hint: Enabling debug output allows you to see unstripped '
173 'values during variable export.') 178​ 'values during variable export.')
174 return 179​ return
▲ Show 20 Lines • Show All 67 Lines • Show Last 20 Lines
View Options

testing/test_resultsdb_directive.py

Show First 20 Lines • Show All 90 Lines • ▼ Show 20 Line(s)22class TestResultsdbReport():
91 def test_failed_tap_import(self, monkeypatch): 91​ def test_failed_tap_import(self, monkeypatch):
92 """Checks if failed TAP import raises exception""" 92​ """Checks if failed TAP import raises exception"""
93 monkeypatch.setattr(check, 'import_TAP', 93​ monkeypatch.setattr(check, 'import_TAP',
94 exception_raiser(TaskotronValueError("Testing Error"))) 94​ exception_raiser(TaskotronValueError("Testing Error")))
95 95​
96 with pytest.raises(TaskotronDirectiveError): 96​ with pytest.raises(TaskotronDirectiveError):
97 self.test_rdb.process(self.ref_input, self.ref_envdata) 97​ self.test_rdb.process(self.ref_input, self.ref_envdata)
98 98​
99​ def test_tap_missing_item(self):
100​ """Checks if missing item raises exception"""
101​ tap = ""
102​ for line in self.ref_input['results'].split('\n'):
103​ if 'item:' in line:
104​ continue
105​ tap += line + '\n'
kparalUnsubmitted
Not Done

Alternatively,

tap = '\n'.join([line for line in self.ref_input['results'].splitlines() if 'item:' not in line])

However, not sure if that's more readable :)

106​
107​ with pytest.raises(TaskotronDirectiveError):
108​ self.test_rdb.process({"results": tap}, self.ref_envdata)
109​
110​ def test_tap_missing_type(self):
111​ """Checks if missing type raises exception"""
112​ tap = ""
113​ for line in self.ref_input['results'].split('\n'):
114​ if 'type:' in line:
115​ continue
116​ tap += line + '\n'
117​
118​ with pytest.raises(TaskotronDirectiveError):
119​ self.test_rdb.process({"results": tap}, self.ref_envdata)
120​
99 def test_report(self): 121​ def test_report(self):
100 """Checks whether TAP is correctly mapped to the reporting method's 122​ """Checks whether TAP is correctly mapped to the reporting method's
101 arguments.""" 123​ arguments."""
102 124​
103 self.test_rdb.process(self.ref_input, self.ref_envdata) 125​ self.test_rdb.process(self.ref_input, self.ref_envdata)
104 126​
105 # Given the input data, the resultsdb should be called once, and only 127​ # Given the input data, the resultsdb should be called once, and only
106 # once, calling "create_result". 128​ # once, calling "create_result".
▲ Show 20 Lines • Show All 83 Lines • Show Last 20 Lines