Address PR comments

conf/jira.yaml.template

@@ -3,6 +3,6 @@ JIRA:
   URL: https://issues.redhat.com
   # Provide api_key to access Jira REST API
   API_KEY: replace-with-jira-api-key
-  COMMENT_TYPE: comment_type
-  COMMENT_VISIBILITY: private_or_public
+  COMMENT_TYPE: group
+  COMMENT_VISIBILITY: "Red Hat Employee"
   ENABLE_COMMENT: false

conftest.py

@@ -2,6 +2,8 @@
 
 import pytest
 
+option = None
+
 pytest_plugins = [
     # Plugins
     'pytest_plugins.auto_vault',
@@ -86,3 +88,10 @@ def pytest_runtest_makereport(item, call):
     # be "setup", "call", "teardown"
 
     setattr(item, "report_" + report.when, report)
+
+
+@pytest.hookimpl(trylast=True)
+def pytest_configure(config):
+    """Make cmdline arguments available."""
+    global option
+    option = config.option

pytest_plugins/jira_comments.py

@@ -1,3 +1,4 @@
+from collections import defaultdict
 import os
 
 import pytest
@@ -24,17 +25,16 @@ def pytest_addoption(parser):
 
 def pytest_configure(config):
     """Register jira_comments markers to avoid warnings."""
-    for marker in [
-        'jira_comments: Add test result comment on Jira issue.',
-    ]:
-        config.addinivalue_line('markers', marker)
+    config.addinivalue_line('markers', 'jira_comments: Add test result comment on Jira issue.')
 
 
 def update_issue_to_tests_map(item, marker, test_result):
+    """If the test has Verifies or BlockedBy doc field,
+    an issue to tests mapping will be added/updated in config.issue_to_tests_map
+    for each test run with outcome of the test.
+    """
     if marker:
         for issue in marker.args[0]:
-            if issue not in item.config.issue_to_tests_map:
-                item.config.issue_to_tests_map[issue] = []
             item.config.issue_to_tests_map[issue].append(
                 {'nodeid': item.nodeid, 'outcome': test_result}
             )
@@ -64,7 +64,7 @@ def pytest_runtest_makereport(item, call):
                 else 'failed'
             )
             if not hasattr(item.config, 'issue_to_tests_map'):
-                item.config.issue_to_tests_map = {}
+                item.config.issue_to_tests_map = defaultdict(list)
             # Update issue_to_tests_map for Verifies testimony marker
             update_issue_to_tests_map(item, verifies_marker, test_result)
             # Update issue_to_tests_map for BlockedBy testimony marker
@@ -78,12 +78,12 @@ def pytest_sessionfinish(session, exitstatus):
         build_url = os.environ.get('BUILD_URL')
         for issue in session.config.issue_to_tests_map:
             comment_body = (
-                f'This is an automated comment from user/job: {build_url if build_url else user} for a Robottelo test run.\n'
+                f'This is an automated comment from job/user: {build_url if build_url else user} for a Robottelo test run.\n'
                 f'Satellite/Capsule: {settings.server.version.release} Snap: {settings.server.version.snap} \n'
                 f'Result for tests linked with issue: {issue} \n'
             )
             for item in session.config.issue_to_tests_map[issue]:
-                comment_body = comment_body + f'{item["nodeid"]} : {item["outcome"]} \n'
+                comment_body += f'{item["nodeid"]} : {item["outcome"]} \n'
             try:
                 add_comment_on_jira(issue, comment_body)
             except Exception as e:

robottelo/config/validators.py

@@ -195,6 +195,9 @@
     jira=[
         Validator('jira.url', default='https://issues.redhat.com'),
         Validator('jira.api_key', must_exist=True),
+        Validator('jira.comment_type', default="group"),
+        Validator('jira.comment_visibility', default="Red Hat Employee"),
+        Validator('jira.enable_comment', default=False),
     ],
     ldap=[
         Validator(

robottelo/utils/issue_handlers/jira.py

@@ -6,6 +6,7 @@
 import requests
 from tenacity import retry, stop_after_attempt, wait_fixed
 
+from conftest import option
 from robottelo.config import settings
 from robottelo.constants import (
     JIRA_CLOSED_STATUSES,
@@ -21,15 +22,15 @@
 VERSION_RE = re.compile(r'(?:sat-)*?(?P<version>\d\.\d)\.\w*')
 
 
-def is_open_jira(issue, data=None):
+def is_open_jira(issue_id, data=None):
     """Check if specific Jira is open consulting a cached `data` dict or
     calling Jira REST API.
 
     Arguments:
-        issue {str} -- The Jira reference e.g: SAT-20548
+        issue_id {str} -- The Jira reference e.g: SAT-20548
         data {dict} -- Issue data indexed by <handler>:<number> or None
     """
-    jira = try_from_cache(issue, data)
+    jira = try_from_cache(issue_id, data)
     if jira.get("is_open") is not None:  # issue has been already processed
         return jira["is_open"]
 
@@ -54,39 +55,39 @@ def is_open_jira(issue, data=None):
     return status not in JIRA_CLOSED_STATUSES and status != JIRA_ONQA_STATUS
 
 
-def are_all_jira_open(issues, data=None):
+def are_all_jira_open(issue_ids, data=None):
     """Check if all Jira is open consulting a cached `data` dict or
     calling Jira REST API.
 
     Arguments:
-        issues {list} -- The Jira reference e.g: ['SAT-20548', 'SAT-20548']
+        issue_ids {list} -- The Jira reference e.g: ['SAT-20548', 'SAT-20548']
         data {dict} -- Issue data indexed by <handler>:<number> or None
     """
-    return all(is_open_jira(issue, data) for issue in issues)
+    return all(is_open_jira(issue_id, data) for issue_id in issue_ids)
 
 
-def are_any_jira_open(issues, data=None):
+def are_any_jira_open(issue_ids, data=None):
     """Check if any of the Jira is open consulting a cached `data` dict or
     calling Jira REST API.
 
     Arguments:
-        issues {list} -- The Jira reference e.g: ['SAT-20548', 'SAT-20548']
+        issue_ids {list} -- The Jira reference e.g: ['SAT-20548', 'SAT-20548']
         data {dict} -- Issue data indexed by <handler>:<number> or None
     """
-    return any(is_open_jira(issue, data) for issue in issues)
+    return any(is_open_jira(issue_id, data) for issue_id in issue_ids)
 
 
-def should_deselect_jira(issue, data=None):
-    """Check if test should be deselected based on marked issue.
+def should_deselect_jira(issue_id, data=None):
+    """Check if test should be deselected based on marked issue_id.
 
     1. Resolution "Obsolete" should deselect
 
     Arguments:
-        issue {str} -- The Jira reference e.g: SAT-12345
+        issue_id {str} -- The Jira reference e.g: SAT-12345
         data {dict} -- Issue data indexed by <handler>:<number> or None
     """
 
-    jira = try_from_cache(issue, data)
+    jira = try_from_cache(issue_id, data)
     if jira.get("is_deselected") is not None:  # issue has been already processed
         return jira["is_deselected"]
 
@@ -105,21 +106,21 @@ def follow_duplicates(jira):
     return jira
 
 
-def try_from_cache(issue, data=None):
+def try_from_cache(issue_id, data=None):
     """Try to fetch issue from given data cache or previous loaded on pytest.
 
     Arguments:
-         issue {str} -- The Jira reference e.g: SAT-12345
+         issue_id {str} -- The Jira reference e.g: SAT-12345
          data {dict} -- Issue data indexed by <handler>:<number> or None
     """
     try:
-        # issue must be passed in `data` argument or already fetched in pytest
-        if not data and not len(pytest.issue_data[issue]['data']):
+        # issue_id must be passed in `data` argument or already fetched in pytest
+        if not data and not len(pytest.issue_data[issue_id]['data']):
             raise ValueError
-        return data or pytest.issue_data[issue]['data']
+        return data or pytest.issue_data[issue_id]['data']
     except (KeyError, AttributeError, ValueError):  # pragma: no cover
         # If not then call Jira API again
-        return get_single_jira(str(issue))
+        return get_single_jira(str(issue_id))
 
 
 def collect_data_jira(collected_data, cached_data):  # pragma: no cover
@@ -169,26 +170,26 @@ def collect_dupes(jira, collected_data, cached_data=None):  # pragma: no cover
     stop=stop_after_attempt(4),  # Retry 3 times before raising
     wait=wait_fixed(20),  # Wait seconds between retries
 )
-def get_data_jira(jira_numbers, cached_data=None):  # pragma: no cover
+def get_data_jira(issue_ids, cached_data=None):  # pragma: no cover
     """Get a list of marked Jira data and query Jira REST API.
 
     Arguments:
-        jira_numbers {list of str} -- ['SAT-12345', ...]
+        issue_ids {list of str} -- ['SAT-12345', ...]
         cached_data {dict} -- Cached data previous loaded from API
 
     Returns:
         [list of dicts] -- [{'id':..., 'status':..., 'resolution': ...}]
     """
-    if not jira_numbers:
+    if not issue_ids:
         return []
 
-    cached_by_call = CACHED_RESPONSES['get_data'].get(str(sorted(jira_numbers)))
+    cached_by_call = CACHED_RESPONSES['get_data'].get(str(sorted(issue_ids)))
     if cached_by_call:
         return cached_by_call
 
     if cached_data:
-        logger.debug(f"Using cached data for {set(jira_numbers)}")
-        if not all([f'{number}' in cached_data for number in jira_numbers]):
+        logger.debug(f"Using cached data for {set(issue_ids)}")
+        if not all([f'{number}' in cached_data for number in issue_ids]):
             logger.debug("There are Jira's out of cache.")
         return [item['data'] for _, item in cached_data.items() if 'data' in item]
 
@@ -200,10 +201,10 @@ def get_data_jira(jira_numbers, cached_data=None):  # pragma: no cover
             "Provide api_key or a jira_cache.json."
         )
         # Provide default data for collected Jira's.
-        return [get_default_jira(number) for number in jira_numbers]
+        return [get_default_jira(issue_id) for issue_id in issue_ids]
 
     # No cached data so Call Jira API
-    logger.debug(f"Calling Jira API for {set(jira_numbers)}")
+    logger.debug(f"Calling Jira API for {set(issue_ids)}")
     jira_fields = [
         "key",
         "summary",
@@ -216,7 +217,7 @@ def get_data_jira(jira_numbers, cached_data=None):  # pragma: no cover
         assert field not in jira_fields
 
     # Generate jql
-    jql = ' OR '.join([f"id = {id}" for id in jira_numbers])
+    jql = ' OR '.join([f"id = {issue_id}" for issue_id in issue_ids])
 
     response = requests.get(
         f"{settings.jira.url}/rest/api/latest/search/",
@@ -244,28 +245,28 @@ def get_data_jira(jira_numbers, cached_data=None):  # pragma: no cover
         for issue in data
         if issue is not None
     ]
-    CACHED_RESPONSES['get_data'][str(sorted(jira_numbers))] = data
+    CACHED_RESPONSES['get_data'][str(sorted(issue_ids))] = data
     return data
 
 
-def get_single_jira(number, cached_data=None):  # pragma: no cover
+def get_single_jira(issue_id, cached_data=None):  # pragma: no cover
     """Call Jira API to get a single Jira data and cache it"""
     cached_data = cached_data or {}
-    jira_data = CACHED_RESPONSES['get_single'].get(number)
+    jira_data = CACHED_RESPONSES['get_single'].get(issue_id)
     if not jira_data:
         try:
-            jira_data = cached_data[f"{number}"]['data']
+            jira_data = cached_data[f"{issue_id}"]['data']
         except (KeyError, TypeError):
-            jira_data = get_data_jira([str(number)], cached_data)
+            jira_data = get_data_jira([str(issue_id)], cached_data)
             jira_data = jira_data and jira_data[0]
-        CACHED_RESPONSES['get_single'][number] = jira_data
-    return jira_data or get_default_jira(number)
+        CACHED_RESPONSES['get_single'][issue_id] = jira_data
+    return jira_data or get_default_jira(issue_id)
 
 
-def get_default_jira(number):  # pragma: no cover
+def get_default_jira(issue_id):  # pragma: no cover
     """This is the default Jira data when it is not possible to reach Jira api"""
     return {
-        "key": number,
+        "key": issue_id,
         "is_open": True,
         "is_deselected": False,
         "status": "",
@@ -275,30 +276,32 @@ def get_default_jira(number):  # pragma: no cover
 
 
 def add_comment_on_jira(
-    jira_number,
+    issue_id,
     comment,
     comment_type=settings.jira.comment_type,
     comment_visibility=settings.jira.comment_visibility,
 ):
     """Adds a new comment to a Jira issue.
 
     Arguments:
-        jira_numbers {str} -- Jira issue number, ex. SAT-12232
+        issue_id {str} -- Jira issue number, ex. SAT-12232
         comment {str}  -- Comment to add on the issue.
         comment_type {str}  -- Type of comment to add.
         comment_visibility {str}  -- Comment visibility.
 
     Returns:
         [list of dicts] -- [{'id':..., 'status':..., 'resolution': ...}]
     """
-    if not settings.jira.enable_comment:
+    # Raise a warning if any of the following option is not set. Note: It's a xor condition.
+    if settings.jira.enable_comment != option.jira_comments:
         logger.warning(
             'Jira comments are currently disabled for this run. '
             'To enable it, please set "enable_comment" to "true" in "config/jira.yaml"'
         )
-    logger.debug(f"Adding a new comment on {jira_number} Jira issue.")
+        return None
+    logger.debug(f"Adding a new comment on {issue_id} Jira issue.")
     response = requests.post(
-        f"{settings.jira.url}/rest/api/latest/issue/{jira_number}/comment",
+        f"{settings.jira.url}/rest/api/latest/issue/{issue_id}/comment",
         json={
             "body": comment,
             "visibility": {