#455 Fix pylint
Merged 4 years ago by gnaponie. Opened 4 years ago by gnaponie.
gnaponie/greenwave fix-pylint  into  master

file modified
+5 -6
@@ -331,6 +331,7 @@ 

  

      @staticmethod

      def process_on_demand_rules(rules):

+         #pylint: disable=attribute-defined-outside-init

Doesn't pylint or flake8 complain about no space after #?

          """

          Validates rules and creates objects for them.

  
@@ -351,8 +352,8 @@ 

                  processed_rules.append(RemoteRule())

              elif rule['type'] == 'PassingTestCaseRule':

                  temp_rule = PassingTestCaseRule()

-                 temp_rule.test_case_name = rule['test_case_name']  # pylint: disable=W0201

-                 temp_rule.scenario = rule.get('scenario')  # pylint: disable=W0201

+                 temp_rule.test_case_name = rule['test_case_name']

+                 temp_rule.scenario = rule.get('scenario')

                  processed_rules.append(temp_rule)

              else:

                  raise BadRequest('Invalid rule type {}'.format(rule['type']))
@@ -493,7 +494,6 @@ 

                      visited_arch_variants.add(arch_variant)

                      answer = self._answer_for_result(

                          result,

-                         product_version,

                          policy.subject_type,

                          subject_identifier)

                      answers.append(answer)
@@ -507,7 +507,6 @@ 

          for result in matching_results:

              answers.append(self._answer_for_result(

                  result,

-                 product_version,

                  policy.subject_type,

                  subject_identifier))

          return answers
@@ -524,7 +523,7 @@ 

          }

  

      def _answer_for_result(

-             self, result, product_version, subject_type, subject_identifier):

+             self, result, subject_type, subject_identifier):

          if result['outcome'] in ('PASSED', 'INFO'):

              log.debug('Test result passed for the result_id %s and testcase %s,'

                        ' because the outcome is %s', result['id'], self.test_case_name,
@@ -681,7 +680,7 @@ 

          policy.relevance_key = data_dict.get('relevance_key')

  

          # Validate the data before processing.

-         policy.__validate_attributes()  # pylint: disable=W0212

+         policy.__validate_attributes()  # pylint: disable=protected-access

          return policy

  

      def __validate_attributes(self):

@@ -1,6 +1,7 @@ 

  import requests

  

  from requests.adapters import HTTPAdapter

+ # pylint: disable=import-error

  from requests.packages.urllib3.util.retry import Retry

  

  from greenwave import __version__

file modified
+1 -1
@@ -196,7 +196,7 @@ 

      cmd = ['git', 'archive', f'--remote={dist_git_url}', rev, 'gating.yaml']

      # Retry thrice if TimeoutExpired exception is raised

      MAX_RETRY = 3

-     for tries in range(MAX_RETRY):

+     for _ in range(MAX_RETRY):

          try:

              git_archive = subprocess.Popen(cmd, stdout=subprocess.PIPE, stderr=subprocess.PIPE)

              output, error_output = git_archive.communicate(timeout=30)

@@ -63,6 +63,8 @@ 

  

  

  def test_make_decision_retrieves_waivers_on_missing(mock_results, mock_waivers):

+     mock_results.return_value = []

+     mock_waivers.return_value = []

      response = make_decision()

      assert 200 == response.status_code

      assert '1 of 1 required test results missing' == response.json['summary']
@@ -71,6 +73,7 @@ 

  

  def test_make_decision_retrieves_waivers_on_failed(mock_results, mock_waivers):

      mock_results.return_value = [make_result(outcome='FAILED')]

+     mock_waivers.return_value = []

      response = make_decision()

      assert 200 == response.status_code

      assert '1 of 1 required tests failed' == response.json['summary']
@@ -79,6 +82,7 @@ 

  

  def test_make_decision_retrieves_waivers_omitted_on_passed(mock_results, mock_waivers):

      mock_results.return_value = [make_result(outcome='PASSED')]

+     mock_waivers.return_value = []

      response = make_decision()

      assert 200 == response.status_code

      assert 'All required tests passed' == response.json['summary']
@@ -86,6 +90,8 @@ 

  

  

  def test_make_decision_retrieves_waivers_once_on_verbose_and_missing(mock_results, mock_waivers):

+     mock_results.return_value = []

+     mock_waivers.return_value = []

      response = make_decision(verbose=True)

      assert 200 == response.status_code

      assert '1 of 1 required test results missing' == response.json['summary']

@@ -912,7 +912,7 @@ 

              scm.return_value = (namespace, 'nethack', 'c3c47a08a66451cb9686c49f040776ed35a0d1bb')

              with mock.patch('greenwave.resources.retrieve_yaml_remote_rule') as f:

                  f.return_value = remote_fragment

-                 policy = OnDemandPolicy.create_from_json(serverside_json)  # pylint: disable=W0212

+                 policy = OnDemandPolicy.create_from_json(serverside_json)

  

                  # Ensure that presence of a result is success.

                  results = DummyResultsRetriever(nvr, 'dist.upgradepath')

@@ -313,7 +313,7 @@ 

  

  

  def test_guess_product_version():

-     # pylint: disable=W0212

+     # pylint: disable=protected-access

      hub = mock.MagicMock()

      hub.config = {

          'environment': 'environment',
@@ -331,6 +331,7 @@ 

  

  

  def test_guess_product_version_with_koji():

+     # pylint: disable=protected-access,unused-variable

      class DummyKojiProxy():

          def getBuild(self, subject_identifier):

              assert 'fake_koji_build' == subject_identifier
@@ -353,6 +354,7 @@ 

      'badnvr-1.2.f30',

  ))

  def test_guess_product_version_failure(nvr):

+     # pylint: disable=protected-access

      product_version = greenwave.consumers.resultsdb._subject_product_version(nvr, 'koji_build')

      assert product_version is None

  

@@ -16,6 +16,7 @@ 

  

  

  def test_waivers_retriever_retrieves_not_ignored_ids():

+     # pylint: disable=protected-access

      retriever = WaiversRetriever(**_DUMMY_RETRIEVER_ARGUMENTS)

      retriever.ignore_ids = [100]

      waiver = dict(
@@ -32,6 +33,7 @@ 

  

  

  def test_waivers_retriever_ignores_ids():

+     # pylint: disable=protected-access

      retriever = WaiversRetriever(**_DUMMY_RETRIEVER_ARGUMENTS)

      retriever.ignore_ids = [99]

      waiver = dict(
@@ -48,6 +50,7 @@ 

  

  

  def test_waivers_retriever_ignores_no_waived():

+     # pylint: disable=protected-access

      retriever = WaiversRetriever(**_DUMMY_RETRIEVER_ARGUMENTS)

      waiver = dict(

          id=99,

file modified
+1 -1
@@ -1,2 +1,2 @@ 

  [MESSAGES CONTROL]

- disable=R,C,no-member,fixme,W0622

+ disable=R,C,no-member,fixme,redefined-builtin,redefined-outer-name

# pylint: disable=import-error

I'm seeing the same issue, but shouldn't it be imported without an issue? Following doesn't give any errors.

python3 -c 'from requests.packages.urllib3.util.retry import Retry'

This won't work because it doesn't mock the resultdb/waiverdb.

Maybe better to move mocked.return_value = [] from above here.

mock_results.return_value = []
mock_waivers.return_value = []

Or maybe use decorators like:

@patch('greenwave.resources.ResultsRetriever.retrieve', return_value=[])

(BTW, I thought pylint skips tests.)

pylint: disable=import-error

I'm seeing the same issue, but shouldn't it be imported without an issue? Following doesn't give any errors.
python3 -c 'from requests.packages.urllib3.util.retry import Retry'

I know! I tried that too and it doesn't give me any errors. Maybe pylint bug? I thought just to disable it.

(BTW, I thought pylint skips tests.)

Yeah it's a bit annoying.

rebased onto 3e30d1a

4 years ago

@lholecek could you please check again?

Can you replace the E* and W* pylint codes with the short identifiers?

1 new commit added

  • Replace pylint codes with verbal short identifiers
4 years ago

Doesn't pylint or flake8 complain about no space after #?

Doesn't pylint or flake8 complain about no space after #?

not if it is for pylint... don't ask me why :D

Commit 26ef3db fixes this pull-request

Pull-Request has been merged by gnaponie

4 years ago

Pull-Request has been merged by gnaponie

4 years ago