util: fix unit-test for GetClusterMappingInfo()

Unit-testing often fails due to a race condition while writing the
clusterMappingConfigFile from multiple go-routines at the same time.
Failures from `make containerized-test` look like this:

    === CONT  TestGetClusterMappingInfo/site2-storage_cluster-id_mapping
        cluster_mapping_test.go:153: GetClusterMappingInfo() = <nil>, expected data &[{map[site1-storage:site2-storage] [map[1:3]] [map[11:5]]} {map[site3-storage:site2-storage] [map[8:3]] [map[10:5]]}]
    === CONT  TestGetClusterMappingInfo/site3-storage_cluster-id_mapping
        cluster_mapping_test.go:153: GetClusterMappingInfo() = <nil>, expected data &[{map[site3-storage:site2-storage] [map[8:3]] [map[10:5]]}]
    --- FAIL: TestGetClusterMappingInfo (0.01s)
        --- PASS: TestGetClusterMappingInfo/mapping_file_not_found (0.00s)
        --- PASS: TestGetClusterMappingInfo/mapping_file_found_with_empty_data (0.00s)
        --- PASS: TestGetClusterMappingInfo/cluster-id_mapping_not_found (0.00s)
        --- FAIL: TestGetClusterMappingInfo/site2-storage_cluster-id_mapping (0.00s)
        --- FAIL: TestGetClusterMappingInfo/site3-storage_cluster-id_mapping (0.00s)
        --- PASS: TestGetClusterMappingInfo/site1-storage_cluster-id_mapping (0.00s)

By splitting the public GetClusterMappingInfo() function into an
internal getClusterMappingInfo() that takes a filename, unit-testing can
use different files for each go-routine, and testing becomes more
predictable.

Signed-off-by: Niels de Vos <ndevos@redhat.com>
This commit is contained in:
Niels de Vos 2021-08-24 17:23:51 +02:00 committed by mergify[bot]
parent b0b46680e3
commit 68588dc7df
2 changed files with 18 additions and 12 deletions

View File

@ -65,9 +65,9 @@ type ClusterMappingInfo struct {
// ... // ...
// }] // }]
func readClusterMappingInfo() (*[]ClusterMappingInfo, error) { func readClusterMappingInfo(filename string) (*[]ClusterMappingInfo, error) {
var info []ClusterMappingInfo var info []ClusterMappingInfo
content, err := ioutil.ReadFile(clusterMappingConfigFile) content, err := ioutil.ReadFile(filename) // #nosec:G304, file inclusion via variable.
if err != nil { if err != nil {
err = fmt.Errorf("error fetching clusterID mapping %w", err) err = fmt.Errorf("error fetching clusterID mapping %w", err)
@ -83,11 +83,11 @@ func readClusterMappingInfo() (*[]ClusterMappingInfo, error) {
return &info, nil return &info, nil
} }
// GetClusterMappingInfo returns corresponding cluster details like clusterID's // getClusterMappingInfo returns corresponding cluster details like clusterID's
// poolID,fscID lists read from configfile. // poolID,fscID lists read from 'filename'.
func GetClusterMappingInfo(clusterID string) (*[]ClusterMappingInfo, error) { func getClusterMappingInfo(clusterID, filename string) (*[]ClusterMappingInfo, error) {
var mappingInfo []ClusterMappingInfo var mappingInfo []ClusterMappingInfo
info, err := readClusterMappingInfo() info, err := readClusterMappingInfo(filename)
if err != nil { if err != nil {
// discard not found error as this file is expected to be created by // discard not found error as this file is expected to be created by
// the admin in case of failover. // the admin in case of failover.
@ -114,3 +114,9 @@ func GetClusterMappingInfo(clusterID string) (*[]ClusterMappingInfo, error) {
return &mappingInfo, nil return &mappingInfo, nil
} }
// GetClusterMappingInfo returns corresponding cluster details like clusterID's
// poolID,fscID lists read from configfile.
func GetClusterMappingInfo(clusterID string) (*[]ClusterMappingInfo, error) {
return getClusterMappingInfo(clusterID, clusterMappingConfigFile)
}

View File

@ -138,19 +138,19 @@ func TestGetClusterMappingInfo(t *testing.T) {
currentTT := tt currentTT := tt
t.Run(currentTT.name, func(t *testing.T) { t.Run(currentTT.name, func(t *testing.T) {
t.Parallel() t.Parallel()
clusterMappingConfigFile = fmt.Sprintf("%s/mapping-%d.json", mappingBasePath, currentI) mappingConfigFile := fmt.Sprintf("%s/mapping-%d.json", mappingBasePath, currentI)
if len(currentTT.mappingFilecontent) != 0 { if len(currentTT.mappingFilecontent) != 0 {
err = ioutil.WriteFile(clusterMappingConfigFile, currentTT.mappingFilecontent, 0o600) err = ioutil.WriteFile(mappingConfigFile, currentTT.mappingFilecontent, 0o600)
if err != nil { if err != nil {
t.Errorf("GetClusterMappingInfo() error = %v", err) t.Errorf("failed to write to %q, error = %v", mappingConfigFile, err)
} }
} }
data, mErr := GetClusterMappingInfo(currentTT.clusterID) data, mErr := getClusterMappingInfo(currentTT.clusterID, mappingConfigFile)
if (mErr != nil) != currentTT.expectErr { if (mErr != nil) != currentTT.expectErr {
t.Errorf("GetClusterMappingInfo() error = %v, expected Error %v", mErr, currentTT.expectErr) t.Errorf("getClusterMappingInfo() error = %v, expected Error %v", mErr, currentTT.expectErr)
} }
if !reflect.DeepEqual(data, currentTT.expectedData) { if !reflect.DeepEqual(data, currentTT.expectedData) {
t.Errorf("GetClusterMappingInfo() = %v, expected data %v", data, currentTT.expectedData) t.Errorf("getClusterMappingInfo() = %v, expected data %v", data, currentTT.expectedData)
} }
}) })
} }