Skip to content

Commit

Permalink
tests: pcidb: never fetch the updated DB from net
Browse files Browse the repository at this point in the history
Should the pcidb pkg fail to find a local pci database, it optionally
fetches an updated copy from the network.
Even though this is unlikely to happen, and even though this is a nice
feature to have, this is enabled by default and can lead to
hard-to-debug issues.

we seen this first with failing CI on xenial in travis (jaypipes#230 (comment))
It's possible the ultimate cause is out of date test VM, or any CI
failure; but it is also true this a symptom of a deeper issue:
test environments should be tightly controlled in order to be reproducible.

Fetching data from the network, even though  from a safe and trusted location,
voids this assumption.
So we need to force this off in the test path.

Unfortunately this patch has its own drawback: it changes the pcidb
default behaviour, so test code and production code have different
preferred code paths.
To really fix this behaviour completely we should probably file a pcidb
issue (+ PR) to change its defaults.

Signed-off-by: Francesco Romani <fromani@redhat.com>
  • Loading branch information
ffromani committed Apr 7, 2021
1 parent b735299 commit 6a0e119
Show file tree
Hide file tree
Showing 4 changed files with 38 additions and 0 deletions.
10 changes: 10 additions & 0 deletions pkg/gpu/gpu_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,16 @@ func TestGPU(t *testing.T) {
if _, ok := os.LookupEnv("GHW_TESTING_SKIP_GPU"); ok {
t.Skip("Skipping GPU tests.")
}

// we seen this first with failing CI on xenial in travis
// However this is a symptom of a deeper issue: test environments should be tightly
// controlled in order to be reproducible. Fetching data from the network, even though
// from a safe and trusted location, voids this assumption.
// So we need to force this off in the test path.
if err := os.Setenv("PCIDB_DISABLE_NETWORK_FETCH", "1"); err != nil {
t.Fatalf("Canoot set PCIDB_DISABLE_NETWORK_FETCH")
}

if _, err := os.Stat("/sys/class/drm"); errors.Is(err, os.ErrNotExist) {
t.Skip("Skipping GPU tests. The environment has no /sys/class/drm directory.")
}
Expand Down
9 changes: 9 additions & 0 deletions pkg/pci/pci_linux_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,15 @@ func pciTestSetup(t *testing.T) *pci.Info {
t.Skip("Skipping PCI tests.")
}

// we seen this first with failing CI on xenial in travis
// However this is a symptom of a deeper issue: test environments should be tightly
// controlled in order to be reproducible. Fetching data from the network, even though
// from a safe and trusted location, voids this assumption.
// So we need to force this off in the test path.
if err := os.Setenv("PCIDB_DISABLE_NETWORK_FETCH", "1"); err != nil {
t.Fatalf("Canoot set PCIDB_DISABLE_NETWORK_FETCH")
}

testdataPath, err := testdata.SnapshotsDirectory()
if err != nil {
t.Fatalf("Expected nil err, but got %v", err)
Expand Down
10 changes: 10 additions & 0 deletions pkg/pci/pci_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,16 @@ func TestPCI(t *testing.T) {
if _, ok := os.LookupEnv("GHW_TESTING_SKIP_PCI"); ok {
t.Skip("Skipping PCI tests.")
}

// we seen this first with failing CI on xenial in travis
// However this is a symptom of a deeper issue: test environments should be tightly
// controlled in order to be reproducible. Fetching data from the network, even though
// from a safe and trusted location, voids this assumption.
// So we need to force this off in the test path.
if err := os.Setenv("PCIDB_DISABLE_NETWORK_FETCH", "1"); err != nil {
t.Fatalf("Canoot set PCIDB_DISABLE_NETWORK_FETCH")
}

info, err := pci.New()
if err != nil {
t.Fatalf("Expected no error creating PciInfo, but got %v", err)
Expand Down
9 changes: 9 additions & 0 deletions pkg/sriov/sriov_linux_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -173,6 +173,15 @@ func sriovTestSetup(t *testing.T) *sriov.Info {
t.Skip("Skipping SRIOV tests.")
}

// we seen this first with failing CI on xenial in travis
// However this is a symptom of a deeper issue: test environments should be tightly
// controlled in order to be reproducible. Fetching data from the network, even though
// from a safe and trusted location, voids this assumption.
// So we need to force this off in the test path.
if err := os.Setenv("PCIDB_DISABLE_NETWORK_FETCH", "1"); err != nil {
t.Fatalf("Canoot set PCIDB_DISABLE_NETWORK_FETCH")
}

testdataPath, err := testdata.SnapshotsDirectory()
if err != nil {
t.Fatalf("Expected nil err, but got %v", err)
Expand Down

0 comments on commit 6a0e119

Please sign in to comment.