Skip to content

[Mellanox][platform] Adjust test case according to get_fan_direction update#3373

Merged
jleveque merged 4 commits intosonic-net:masterfrom
stephenxs:test_new_fan_dir_gidhub
May 8, 2021
Merged

[Mellanox][platform] Adjust test case according to get_fan_direction update#3373
jleveque merged 4 commits intosonic-net:masterfrom
stephenxs:test_new_fan_dir_gidhub

Conversation

@stephenxs
Copy link
Contributor

@stephenxs stephenxs commented Apr 21, 2021

Description of PR

Summary:
Adjust test case according to get_fan_direction update
This PR depends on sonic-net/sonic-buildimage#7386

Signed-off-by: Stephen Sun [email protected]

Type of change

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

Approach

What is the motivation for this PR?

Adjust test case according to get_fan_direction update

How did you do it?

Replace the old way to mock fan direction with the new way.

  • The old way: fan direction is fetched via reading /var/run/hw-management/system/fan_dir, supported on Spetrum-2/3 platforms.
  • The new way: fan direction is fetched via reading /var/run/hw-management/thermal/fanX_dir, supported on all platforms.

How did you verify/test it?

Run regression test.

Any platform specific information?

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

Documentation

@lgtm-com
Copy link

lgtm-com bot commented Apr 21, 2021

This pull request introduces 1 alert when merging 52dc540 into 1e30b38 - view on LGTM.com

new alerts:

  • 1 for Variable defined multiple times

Signed-off-by: Stephen Sun <[email protected]>
@stephenxs
Copy link
Contributor Author

This pull request introduces 1 alert when merging 52dc540 into 1e30b38 - view on LGTM.com

new alerts:

  • 1 for Variable defined multiple times

Fixed.

@stephenxs stephenxs marked this pull request as ready for review April 26, 2021 00:44
@stephenxs stephenxs requested a review from jleveque as a code owner April 26, 2021 00:44
@liat-grozovik
Copy link
Collaborator

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@liat-grozovik
Copy link
Collaborator

will the test know how to run with an old image which is not pending on the new PR you are referring to?

@stephenxs
Copy link
Contributor Author

will the test know how to run with an old image which is not pending on the new PR you are referring to?

No. I assume the pending PR should be merged before the test can run.
Do we need to support run the new test without it? I think we don't need to if it will be cherry-picked to 202012.

@liat-grozovik
Copy link
Collaborator

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

Signed-off-by: Stephen Sun <[email protected]>
@stephenxs
Copy link
Contributor Author

will the test know how to run with an old image which is not pending on the new PR you are referring to?

I just realized that this test can run against 201911. Just update it to tolerance it.

@stephenxs stephenxs closed this Apr 30, 2021
@stephenxs stephenxs reopened this Apr 30, 2021
@stephenxs stephenxs requested a review from liat-grozovik April 30, 2021 07:59
@jleveque
Copy link
Contributor

jleveque commented May 6, 2021

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@stephenxs
Copy link
Contributor Author

Hi @jleveque can we have this PR merged as all tests passed? thanks

@jleveque jleveque merged commit 71301e1 into sonic-net:master May 8, 2021
@stephenxs stephenxs deleted the test_new_fan_dir_gidhub branch May 8, 2021 00:25
vmittal-msft pushed a commit to vmittal-msft/sonic-mgmt that referenced this pull request Sep 28, 2021
…update (sonic-net#3373)

Adjust test case according to get_fan_direction update

Signed-off-by: Stephen Sun <[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.

3 participants