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.

Sign up for GitHub

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

Jump to bottom

Remove Arista products from sku-sensors-data.yml #9415

Merged
merged 1 commit into from
Aug 16, 2023
Merged

Remove Arista products from sku-sensors-data.yml #9415

merged 1 commit into from
Aug 16, 2023

Conversation

Staphylo
Copy link
Contributor

Description of PR

The platform_tests/test_sensors.py rely on the information provided in this config file to check for the existance of sysfs paths.

This test was introduced before the Platform API existed and did have some purpose then. However all SONiC platform daemons now rely on the Platform API which is tested by numerous tests under platform_tests.

There is no longer a need to hardcode sysfs paths for products. Keeping this data there is bound to generate recurring issues in the future and translate directly into maintenance burden.

Some sysfs paths are just not deterministic. They will depend on which driver is loaded first and whatnot which is inherently flaky for a test to rely on.

Summary: Remove Arista products from sku-sensors-data.yml
Fixes # (issue)

Type of change

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

Back port request

  • 201911
  • 202012
  • 202205

Approach

What is the motivation for this PR?

Reduce unreliability and maintenance burden on features that are already covered by Platform API tests

How did you do it?

Removed Arista devices from sku-sensors-data.yml

How did you verify/test it?

Ran the test and ensured it is now marked as SKIPPED

Any platform specific information?

Only concerning Arista devices

@Staphylo
Remove Arista products from sku-sensors-data.yml
a3079af 
The `platform_tests/test_sensors.py` rely on the information provided
in this config file to check for the existance of sysfs paths.

This test was introduced before the Platform API existed and did have
some purpose then. However all SONiC platform daemons now rely on the
Platform API which is tested by numerous tests under `platform_tests`.

There is no longer a need to hardcode sysfs paths for products.
Keeping this data there is bound to generate recurring issues in the
future and translate directly into maintenance burden.

Some sysfs paths are just not deterministic. They will depend on which
driver is loaded first and whatnot which is inherently flaky for a test
to rely on.
@lipxu
Copy link
Contributor

lipxu commented Aug 16, 2023

Tried it on 7060device,
After removing the sensor data under x86_64-arista_7060_cx32s: in ../ansible/group_vars/sonic/sku-sensors-data.yml file.
the case could pass on 7060 devices

@lipxu lipxu self-requested a review August 16, 2023 05:58
@lipxu lipxu merged commit 94369b1 into sonic-net:master Aug 16, 2023
11 checks passed
mssonicbld pushed a commit to mssonicbld/sonic-mgmt that referenced this pull request Aug 16, 2023
@Staphylo @mssonicbld
Remove Arista products from sku-sensors-data.yml (sonic-net#9415)
6bb8f19 
The `platform_tests/test_sensors.py` rely on the information provided
in this config file to check for the existance of sysfs paths.

This test was introduced before the Platform API existed and did have
some purpose then. However all SONiC platform daemons now rely on the
Platform API which is tested by numerous tests under `platform_tests`.

There is no longer a need to hardcode sysfs paths for products.
Keeping this data there is bound to generate recurring issues in the
future and translate directly into maintenance burden.

Some sysfs paths are just not deterministic. They will depend on which
driver is loaded first and whatnot which is inherently flaky for a test
to rely on.
@mssonicbld
Copy link
Collaborator

Cherry-pick PR to 202205: #9500

mssonicbld pushed a commit to mssonicbld/sonic-mgmt that referenced this pull request Aug 16, 2023
@Staphylo @mssonicbld
Remove Arista products from sku-sensors-data.yml (sonic-net#9415)
e642dd2 
The `platform_tests/test_sensors.py` rely on the information provided
in this config file to check for the existance of sysfs paths.

This test was introduced before the Platform API existed and did have
some purpose then. However all SONiC platform daemons now rely on the
Platform API which is tested by numerous tests under `platform_tests`.

There is no longer a need to hardcode sysfs paths for products.
Keeping this data there is bound to generate recurring issues in the
future and translate directly into maintenance burden.

Some sysfs paths are just not deterministic. They will depend on which
driver is loaded first and whatnot which is inherently flaky for a test
to rely on.
@mssonicbld
Copy link
Collaborator

Cherry-pick PR to 202305: #9501

@mssonicbld
Copy link
Collaborator

@Staphylo PR conflicts with 202012 branch

This was referenced Aug 16, 2023
Merged
Merged
mssonicbld pushed a commit that referenced this pull request Aug 17, 2023
@Staphylo @mssonicbld
Remove Arista products from sku-sensors-data.yml (#9415)
2f77cbd 
The `platform_tests/test_sensors.py` rely on the information provided
in this config file to check for the existance of sysfs paths.

This test was introduced before the Platform API existed and did have
some purpose then. However all SONiC platform daemons now rely on the
Platform API which is tested by numerous tests under `platform_tests`.

There is no longer a need to hardcode sysfs paths for products.
Keeping this data there is bound to generate recurring issues in the
future and translate directly into maintenance burden.

Some sysfs paths are just not deterministic. They will depend on which
driver is loaded first and whatnot which is inherently flaky for a test
to rely on.
mssonicbld pushed a commit that referenced this pull request Aug 17, 2023
@Staphylo @mssonicbld
Remove Arista products from sku-sensors-data.yml (#9415)
25b8f30 
The `platform_tests/test_sensors.py` rely on the information provided
in this config file to check for the existance of sysfs paths.

This test was introduced before the Platform API existed and did have
some purpose then. However all SONiC platform daemons now rely on the
Platform API which is tested by numerous tests under `platform_tests`.

There is no longer a need to hardcode sysfs paths for products.
Keeping this data there is bound to generate recurring issues in the
future and translate directly into maintenance burden.

Some sysfs paths are just not deterministic. They will depend on which
driver is loaded first and whatnot which is inherently flaky for a test
to rely on.
Staphylo added a commit to Staphylo/sonic-mgmt that referenced this pull request Aug 21, 2023
@Staphylo
Remove Arista products from sku-sensors-data.yml (sonic-net#9415)
b48c36a 
The `platform_tests/test_sensors.py` rely on the information provided
in this config file to check for the existance of sysfs paths.

This test was introduced before the Platform API existed and did have
some purpose then. However all SONiC platform daemons now rely on the
Platform API which is tested by numerous tests under `platform_tests`.

There is no longer a need to hardcode sysfs paths for products.
Keeping this data there is bound to generate recurring issues in the
future and translate directly into maintenance burden.

Some sysfs paths are just not deterministic. They will depend on which
driver is loaded first and whatnot which is inherently flaky for a test
to rely on.
@Staphylo Staphylo mentioned this pull request Aug 21, 2023
Merged
3 tasks
@Staphylo
Copy link
Contributor Author

@lipxu I opened #9569 to address the cherry-pick conflict on 202012

lipxu pushed a commit that referenced this pull request Aug 21, 2023
@Staphylo
Remove Arista products from sku-sensors-data.yml (#9415) (#9569)
1826b54 
The `platform_tests/test_sensors.py` rely on the information provided
in this config file to check for the existance of sysfs paths.

This test was introduced before the Platform API existed and did have
some purpose then. However all SONiC platform daemons now rely on the
Platform API which is tested by numerous tests under `platform_tests`.

There is no longer a need to hardcode sysfs paths for products.
Keeping this data there is bound to generate recurring issues in the
future and translate directly into maintenance burden.

Some sysfs paths are just not deterministic. They will depend on which
driver is loaded first and whatnot which is inherently flaky for a test
to rely on.
@lipxu
Copy link
Contributor

lipxu commented Aug 21, 2023

@lipxu I opened #9569 to address the cherry-pick conflict on 202012

Merged to 202012

@lipxu lipxu mentioned this pull request Sep 4, 2023
Open
AharonMalkin pushed a commit to AharonMalkin/sonic-mgmt that referenced this pull request Jan 25, 2024
@Staphylo @AharonMalkin
Remove Arista products from sku-sensors-data.yml (sonic-net#9415)
ed7d254 
The `platform_tests/test_sensors.py` rely on the information provided
in this config file to check for the existance of sysfs paths.

This test was introduced before the Platform API existed and did have
some purpose then. However all SONiC platform daemons now rely on the
Platform API which is tested by numerous tests under `platform_tests`.

There is no longer a need to hardcode sysfs paths for products.
Keeping this data there is bound to generate recurring issues in the
future and translate directly into maintenance burden.

Some sysfs paths are just not deterministic. They will depend on which
driver is loaded first and whatnot which is inherently flaky for a test
to rely on.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lipxu lipxu lipxu approved these changes

Assignees
No one assigned
Labels
Approved for 202012 branch Approved for 202205 branch Approved for 202305 branch Cherry Pick Conflict_202012 Included in 202012 branch Included in 202205 branch Included in 202305 branch Request for 202012 branch Request for 202205 branch Request for 202305 branch
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@Staphylo @lipxu @mssonicbld @wangxin