SatelliteQE / robottelo

Robottelo is a test suite that exercises The Foreman.
GNU General Public License v3.0
61 stars 112 forks source link

bz_cache.json missing data about bugzilla #7685

Closed mirekdlugosz closed 4 years ago

mirekdlugosz commented 4 years ago

tests/foreman/cli/test_hammer.py::HammerCommandsTestCase::test_positive_all_options failed on automation-upgraded-6.6-all-tiers-rhel7 job #18 with following traceback:

self = <tests.foreman.cli.test_hammer.HammerCommandsTestCase testMethod=test_positive_all_options>

    @tier1
    @upgrade
    def test_positive_all_options(self):
        """check all provided options for every hammer command

        :id: 1203ab9f-896d-4039-a166-9e2d36925b5b

        :expectedresults: All expected options are present

        :CaseImportance: Critical
        """
        self.maxDiff = None
> self._traverse_command_tree()

tests/foreman/cli/test_hammer.py:158:
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _
tests/foreman/cli/test_hammer.py:118: in _traverse_command_tree
    if is_open('BZ:1666687'):
robottelo/helpers.py:782: in is_open
    return handlers[handler_code.strip()](issue.strip(), data)
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _

issue = 'BZ:1666687', data = None

    def is_open_bz(issue, data=None):
        """Check if specific BZ is open consulting a cached `data` dict or
        calling Bugzilla REST API.

        Arguments:
            issue {str} -- The BZ reference e.g: BZ:123456
            data {dict} -- Issue data indexed by <handler>:<number> or None
        """

        bz = try_from_cache(issue, data)
        if bz.get("is_open") is not None: # bug has been already processed
            return bz["is_open"]

        bz = follow_duplicates(bz)

        # BZ is explicitly in OPEN status
> if bz['status'] in OPEN_STATUSES:
E KeyError: 'status'

robottelo/issue_handlers/bugzilla.py:41: KeyError

bz issue collector fails to collect data about BZ1666687. I can reproduce issue locally:

$ pytest -s -v tests/foreman/cli/test_hammer.py -k test_positive_all_options
(... snip)
$ cat bz_cache.json 
{
    "_meta": {
        "version": "6.6.2",
        "hostname": "dhcp-3-232.vms.sat.rdu2.redhat.com",
        "created": "2020-03-11T15:57:54.854981",
        "pytest": {
            "args": [
                "tests/foreman/cli/test_hammer.py"
            ],
            "pwd": "/home/mzalewsk/sources/robottelo"
        }
    }
}

For whatever reason, it does work on current master branch. I do not see any significant changes between branches in relevant files.

rochacbruno commented 4 years ago

We could just use .get on if bz['status'] in OPEN_STATUSES: so we avoid the problem, but I am afraid it is NOT a good idea, because if we go on that way we would be silencing those errors.

I think the best now would be going with implementing a proper serializer for BZ

from pydantic import BaseModel

class BZ(BaseModel):
   id: int
   status: str
   target_milestone: str
   ...

bz = BZ(**data_from_bugzilla_api)

Then we would have a ValidationError from pydantic.BaseModel and then we can handle and log that error properly.

Anyway we need to debug the is_open_bz function to see what is happening there.

mirekdlugosz commented 4 years ago

After very fruitful debugging session with @rochacbruno , we were able to identify root cause: in robottelo/issue_handlers/bugzilla.py, try_from_cache line 126 is:

return data or pytest.issue_data[issue]['data']

It's assumed that pytest.issue_data[issue]['data'] will raise KeyError exception, which in turn will execute line 129 which will get data about bugzilla.

Instead, pytest.issue_data[issue] is:

{'data': {}, 'used_in': []}

This causes try_from_cache to return empty dict instead of downloading data about missing bugzilla and returning that.

I will investigate further and raise PR.

rochacbruno commented 4 years ago

@mirekdlugosz something like this?

def try_from_cache(issue, data=None):
    """Try to fetch issue from given data cache or previous loaded on pytest.
   Arguments:
        issue {str} -- The BZ reference e.g: BZ:123456
        data {dict} -- Issue data indexed by <handler>:<number> or None
    """
    try:
        # issue must be passed in `data` argument or already fetched in pytest
        result = data or pytest.issue_data[issue]['data']
    except (KeyError, AttributeError):
        result = None        

    # If not then call BZ API again
    return result or get_single_bz(str(issue).partition(':')[-1])
mirekdlugosz commented 4 years ago

@rochacbruno Something like that could work, but I decided to investigate why it works on master, but fails on 6.6.z. Turned out that robottelo/helpers.py has some extra code in master, which I was able to track to 2d47a3e260565ebe7e4029a7537bf383252ebe39 - merged in #7465. I did see that before, but I ignored it as seemingly unrelated (at least commit title did not make it clear there are changes to bugzilla-gathering logic as well). After cherry-picking that commit to 6.6.z, my affected test passed.

Raised #7687 with cherry-pick as fix for this.