Skip to content

[sfps] Fix issue where the test_sfps failed due to key error#4250

Merged
Blueve merged 3 commits intosonic-net:masterfrom
Blueve:dev/jika/sfpsfix
Sep 15, 2021
Merged

[sfps] Fix issue where the test_sfps failed due to key error#4250
Blueve merged 3 commits intosonic-net:masterfrom
Blueve:dev/jika/sfpsfix

Conversation

@Blueve
Copy link
Collaborator

@Blueve Blueve commented Sep 14, 2021

Description of PR

Summary:
Fix issue where the test_sfps failed due to key error.
The issue was cause by API change and it was introduced by this PR: #4157

Type of change

  • Bug fix
  • Testbed and Framework(new/improvement)
  • Test case(new/improvement)

Back port request

  • 201911

Approach

What is the motivation for this PR?

Fix issue where the test_sfps failed due to key error.

How did you do it?

Convert the dictionary to indices list

How did you verify/test it?

Run test_sfps on a physical DUT

=== Running tests in groups ===
/usr/local/lib/python2.7/dist-packages/ansible/parsing/vault/__init__.py:44: CryptographyDeprecationWarning: Python 2 is no longer supported by the Python core team. Support for it is now deprecated in cryptography, and will be removed in a future release.
  from cryptography.exceptions import InvalidSignature
============================= test session starts ==============================
platform linux2 -- Python 2.7.17, pytest-4.6.5, py-1.9.0, pluggy-0.13.1
ansible: 2.8.12
rootdir: /var/src/internal/tests, inifile: pytest.ini
plugins: forked-1.3.0, xdist-1.28.0, html-1.22.1, metadata-1.10.0, repeat-0.9.1, ansible-2.2.2
collected 1 item                                                               


platform_tests/api/test_chassis.py::TestChassisApi::test_sfps[str2-7215-acs-1] 
PASSED [100%]


----------- generated xml file: /var/src/internal/tests/logs/tr.xml ------------
========================== 1 passed in 134.08 seconds ==========================
INFO:root:Can not get Allure report URL. Please check logs

Any platform specific information?

Supported testbed topology if it's a new test case?

Documentation

@Blueve Blueve requested a review from prgeor September 14, 2021 01:19
@Blueve Blueve requested a review from sujinmkang as a code owner September 14, 2021 01:19
@Blueve Blueve requested a review from yxieca September 14, 2021 01:41
@Blueve Blueve requested a review from prgeor September 14, 2021 03:16
@Blueve
Copy link
Collaborator Author

Blueve commented Sep 14, 2021

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@vdahiya12
Copy link
Contributor

Hi @Blueve , can you integrate your change with #4226.
Hi @prgeor we would still require the fix for num_lens in #4226, because this change will fail for 7050cx3-C48D8 SKU, correct ?

@Blueve
Copy link
Collaborator Author

Blueve commented Sep 15, 2021

Hi @Blueve , can you integrate your change with #4226.
Hi @prgeor we would still require the fix for num_lens in #4226, because this change will fail for 7050cx3-C48D8 SKU, correct ?

@vdahiya12 Do you know why we add len(list_sfps)?
According to https://github.com/Azure/sonic-mgmt/blob/a37b5c1482b9120916655720758df2faf4377c84/tests/platform_tests/api/test_chassis.py#L395
I think if the length of list_sfps is not aligned with num_sfps, it will fail earlier

@prgeor
Copy link
Contributor

prgeor commented Sep 15, 2021

Hi @Blueve , can you integrate your change with #4226.
Hi @prgeor we would still require the fix for num_lens in #4226, because this change will fail for 7050cx3-C48D8 SKU, correct ?

@vdahiya12 suggest you to fix the len issue in your PR #4226 and let this PR fix the 'key_error'. We can discuss on len issue.

@Blueve
Copy link
Collaborator Author

Blueve commented Sep 15, 2021

I am going to check-in this fix first. Let's address other issue in another thread

@Blueve Blueve merged commit 7c947a7 into sonic-net:master Sep 15, 2021
@Blueve Blueve deleted the dev/jika/sfpsfix branch September 15, 2021 02:33
vmittal-msft pushed a commit to vmittal-msft/sonic-mgmt that referenced this pull request Sep 28, 2021
…et#4250)

* [sfps] Fix issue where the test_sfps failed due to key error

Signed-off-by: Jing Kan [email protected]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants