sonic-net / sonic-mgmt

Configuration management examples for SONiC
Other
176 stars 700 forks source link

[test_sfputil] platform_tests.sfp.test_sfputil failed because loganalyzer parameter is None #6491

Open ZhaohuiS opened 1 year ago

ZhaohuiS commented 1 year ago

Description

Recently, platform_tests.sfp.test_sfputil failed with the below error log, it's introduced by PR https://github.com/sonic-net/sonic-mgmt/pull/5301. From the log, we can see loganalyzer is None.

___ ERROR at setup of test_check_sfputil_presence[str2-msn4600c-acs-03-None] ___

duthost = <MultiAsicSonicHost str2-msn4600c-acs-03>, loganalyzer = None

    @pytest.fixture(autouse=True)
    def update_la_ignore_errors_list_for_mlnx(duthost, loganalyzer):
        if duthost.facts["asic_type"] in ["mellanox"]:
>           for host in loganalyzer:
E           TypeError: 'NoneType' object is not iterable

duthost = loganalyzer = None

Steps to reproduce the issue:

  1. Run platform_tests/sfp/test_sfputil.py

Describe the results you received:

Describe the results you expected:

Additional information you deem important:

**Output of `show version`:**

```
(paste your output here)
```

**Attach debug file `sudo generate_dump`:**

```
(paste your output here)
```
ZhaohuiS commented 1 year ago

@ppikh Could you please take a look at this issue? It's caused by PR #5301. Thanks.

ZhaohuiS commented 1 year ago

@ppikh I reverted #5301 in this PR https://github.com/sonic-net/sonic-mgmt/pull/6493 for now, if you fix this issue, please commit it again. Thanks.

ppikh commented 1 year ago

Hi @wangxin , @ZhaohuiS I do not see any reason why test fail with error: TypeError: 'NoneType' object is not iterable Only one reason possible in case when you run test --disable_loganalyzer argument, but it does not make sense in current test.

@ZhaohuiS - could you please provide command which you used for run test?

ZhaohuiS commented 1 year ago

@ppikh It's easy to reproduce, the command is same with the way I run other test cases: pytest platform_tests/sfp/test_sfputil.py -k test_check_sfputil_reset --inventory ../ansible/veos,../ansible/str --host-pattern all --testbed_file ../ansible/testbed.yaml --testbed vms11-2-t0 --log-cli-level debug --showlocals --assert plain --show-capture no -rav --skip_sanity --completeness_level=debug --pdb

ZhaohuiS commented 1 year ago

@ppikh Could you please review this fix in this PR https://github.com/sonic-net/sonic-mgmt/pull/6532? Thanks.

ZhaohuiS commented 1 year ago

@ppikh I rethink about it again. Maybe I should close #6532. Because for test_sfputil.py, it disables loganalyzer in pytest mark, that's why loganalyzer parameter is None in update_la_ignore_errors_list_for_mlnx. If I remove disable-loganalyzer, it will open loganalzyer for other platforms which don't expect to enable it and will cause major failures for this script. I think we should keep https://github.com/sonic-net/sonic-mgmt/pull/6493 active, if mellanox want to enable loganalzyer in its case, then init loganzlyer in its fixture, don't enable loganalyzer globally.