Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fixed startup of QoS SAI tests #5605

Closed

Conversation

antonptashnik
Copy link
Contributor

Description of PR

Summary: fixed startup of QoS SAI tests
Fixes # (issue)
QoS SAI tests fail with the error on ptfhost_utils.py:185 :

TypeError: <lambda>() takes exactly 2 arguments (1 given)

Fixed it

Type of change

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

Back port request

  • 201911
  • 202012

Approach

What is the motivation for this PR?

Fix QoS SAI tests

How did you do it?

Fixed syntax issue in map func usage

How did you verify/test it?

py.test --inventory=../ansible/lab,../ansible/veos --testbed_file=../ansible/testbed.csv --module-path=../ansible/library -v -rA --topology=t1,any qos/test_qos_sai.py

Any platform specific information?

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

Documentation

@antonptashnik antonptashnik requested a review from a team as a code owner May 6, 2022 09:25
Copy link
Contributor

@ZhaohuiS ZhaohuiS left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@antonptashnik
Copy link
Contributor Author

@wangxin take a look please

@ZhaohuiS
Copy link
Contributor

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@ZhaohuiS
Copy link
Contributor

ZhaohuiS commented Jun 21, 2022

@OleksandrKozodoi Does this fix work for both python2 and python3?
I think this is compatibility issue introduced by #4867. For now, sonic-mgmt asks all the changes need to be both python2&3 compatible.
BTW, lambda has different behavior for py2 and py3, is it better to fix it in a common way such as using for loop to replace lambda? Then it could no more compatibility issue for this case. Thanks.
And map doesn't return list in python3, you have to turn it into list.
So, I suggest rewriting the code, don't use lambda and map. What do you think about it?

@antonptashnik
Copy link
Contributor Author

@ZhaohuiS I agree. I'm closing this PR in favor of #5845

@OleksandrKozodoi
Copy link
Contributor

Hi @ZhaohuiS. You are right, this is compatibility issue introduced by #4867. I agree, that it is better to fix it by using a loop. Thanks for your suggestion!

@ZhaohuiS
Copy link
Contributor

Hi @ZhaohuiS. You are right, this is compatibility issue introduced by #4867. I agree, that it is better to fix it by using a loop. Thanks for your suggestion!

@OleksandrKozodoi Thanks.

ZhaohuiS added a commit that referenced this pull request Jun 22, 2022
What is the motivation for this PR?
Fix the issue reported in #4867, failures: TypeError: () takes exactly 2 arguments (1 given)
The fix in #5605 still uses lambda which is not easy to read and may have compatibility issue.

How did you do it?
Use for loop and concentrate string together and append them to a list.

How did you verify/test it?
run tests/test_qos_sai.py

Signed-off-by: Zhaohui Sun <[email protected]>
@antonptashnik antonptashnik deleted the fix-qos-sai-test-startup branch June 22, 2022 08:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants