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

supply override PCIDB_PATH env var in unit tests #376

Merged
merged 1 commit into from
Sep 20, 2024
Merged

Conversation

jaypipes
Copy link
Owner

Some of the unit tests inspect snapshot filesystems and for the GPU Info-collection, the github.com/jaypipes/pcidb library is used to get PCI information. That library no longer (as of version v0.9) does an automatic fetch of the canonical pci.ids file if it cannot find a pci.ids file in the local filesystem. It no longer does that network fetch because the upstream maintainer of the pci.ids database noticed too many fetches coming from projects that imported ghw which transitively imported pcidb.

Because of this change in behaviour of no longer doing an automatic network fetch of the pci.ids database, a unit test in ghw that was relying on the pcidb library was no longer working. This patch fixes two things related to the PCI handling in the pkg/gpu package. First, it cleans up the parsing of the /sys/class/drm symlink contents by looking through the path parts and attempting a regex match for a PCI address. Second, it adds an env-var override to the GPU unit tests to point github.com/jaypipes/pcidb to a specific pci.ids file that I've included in the testdata/ directory.

Closes Issue #375

Some of the unit tests inspect snapshot filesystems and for the GPU
Info-collection, the `github.com/jaypipes/pcidb` library is used to get
PCI information. That library no longer (as of version v0.9) does an
automatic fetch of the canonical pci.ids file if it cannot find a
pci.ids file in the local filesystem. It no longer does that network
fetch because the upstream maintainer of the pci.ids database noticed
too many fetches coming from projects that imported `ghw` which
transitively imported `pcidb`.

Because of this change in behaviour of no longer doing an automatic
network fetch of the pci.ids database, a unit test in `ghw` that was
relying on the pcidb library was no longer working. This patch fixes two
things related to the PCI handling in the `pkg/gpu` package. First, it
cleans up the parsing of the `/sys/class/drm` symlink contents by
looking through the path parts and attempting a regex match for a PCI
address. Second, it adds an env-var override to the GPU unit tests to
point `github.com/jaypipes/pcidb` to a specific pci.ids file that I've
included in the `testdata/` directory.

Closes Issue #375

Signed-off-by: Jay Pipes <[email protected]>
@jaypipes
Copy link
Owner Author

@ffromani this one fixes up a nagging failure in unit tests that I found..

Copy link
Collaborator

@ffromani ffromani left a comment

Choose a reason for hiding this comment

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

I think we can improve further later on.
e.g. I don't like too much os.Stat + SafeIntFromFile - I'll try to improve.
But indeed, later on: this work LGTM and fixes the issue, so I'm fine merging.

@jaypipes
Copy link
Owner Author

I think we can improve further later on. e.g. I don't like too much os.Stat + SafeIntFromFile - I'll try to improve.

totally agreed :) I was actually going in to do some cleanup in that area when I ran into this. I'll be pushing up some cleanup PRs later this weekend!

@jaypipes
Copy link
Owner Author

I've got to fix up the GH action workflows since they are stuck... will do that in a separate PR...

@jaypipes jaypipes merged commit 67825f0 into main Sep 20, 2024
7 of 14 checks passed
@ffromani ffromani deleted the fix-gpu-test branch September 20, 2024 14:40
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.

2 participants