Go updates for the test directory #658
Conversation
| @@ -127,8 +127,6 @@ func GetConfig(datapath string, configType uint32) pfcpiface.Conf { | |||
| } | |||
|
|
|||
| panic("Wrong datapath or config type provided") | |||
There was a problem hiding this comment.
should we return some error instead than panic?
There was a problem hiding this comment.
I think the panic was originally added to stop the test and avoid it to keep running. Below is a way to avoid the panic. What do you think?
diff --git a/test/integration/conf.go b/test/integration/conf.go
index a6c0d62..e6d3165 100644
--- a/test/integration/conf.go
+++ b/test/integration/conf.go
@@ -6,6 +6,7 @@ package integration
import (
"bytes"
"encoding/json"
+ "fmt"
"github.com/omec-project/upf-epc/pfcpiface"
"github.com/sirupsen/logrus"
"net/http"
@@ -106,27 +107,27 @@ func UP4ConfigWipeOutOnUP4Restart() pfcpiface.Conf {
return config
}
-func GetConfig(datapath string, configType uint32) pfcpiface.Conf {
+func GetConfig(datapath string, configType uint32) (pfcpiface.Conf, error) {
switch datapath {
case DatapathUP4:
switch configType {
case ConfigDefault:
- return UP4ConfigDefault()
+ return UP4ConfigDefault(), nil
case ConfigUPFBasedIPAllocation:
- return UP4ConfigUPFBasedIPAllocation()
+ return UP4ConfigUPFBasedIPAllocation(), nil
case ConfigWipeOutOnUP4Restart:
- return UP4ConfigWipeOutOnUP4Restart()
+ return UP4ConfigWipeOutOnUP4Restart(), nil
}
case DatapathBESS:
switch configType {
case ConfigDefault:
- return BESSConfigDefault()
+ return BESSConfigDefault(), nil
case ConfigUPFBasedIPAllocation:
- return BESSConfigUPFBasedIPAllocation()
+ return BESSConfigUPFBasedIPAllocation(), nil
}
}
- panic("Wrong datapath or config type provided")
+ return pfcpiface.Conf{}, fmt.Errorf("wrong datapath or config type provided")
}
func PushSliceMeterConfig(sliceConfig pfcpiface.NetworkSlice) error {
diff --git a/test/integration/framework.go b/test/integration/framework.go
index ebaef89..bc6f48e 100644
--- a/test/integration/framework.go
+++ b/test/integration/framework.go
@@ -327,21 +327,26 @@ func setup(t *testing.T, configType uint32) {
MustStartMockUP4()
}
+ getConfig, err := GetConfig(os.Getenv(EnvDatapath), configType)
+ if err != nil {
+ t.Fatal("Wrong datapath or config type provided")
+ }
+
switch os.Getenv(EnvMode) {
case ModeDocker:
- jsonConf, _ := json.Marshal(GetConfig(os.Getenv(EnvDatapath), configType))
+ jsonConf, _ := json.Marshal(getConfig)
err := os.WriteFile(ConfigPath, jsonConf, os.ModePerm)
require.NoError(t, err)
MustStartPFCPAgent()
case ModeNative:
- pfcpAgent = pfcpiface.NewPFCPIface(GetConfig(os.Getenv(EnvDatapath), configType))
+ pfcpAgent = pfcpiface.NewPFCPIface(getConfig)
go pfcpAgent.Run()
default:
t.Fatal("Unexpected test mode")
}
pfcpClient = pfcpsim.NewPFCPClient("127.0.0.1")
- err := pfcpClient.ConnectN4("127.0.0.1")
+ err = pfcpClient.ConnectN4("127.0.0.1")
require.NoErrorf(t, err, "failed to connect to UPF")
// wait for PFCP Agent to initialize, blockingThere was a problem hiding this comment.
@badhrinathpa, any comment on this?
| @@ -246,22 +238,10 @@ func waitForBESSFakeToStart() error { | |||
| return waitForPortOpen("tcp", "127.0.0.1", "10514") | |||
| } | |||
|
|
|||
There was a problem hiding this comment.
what were these functions used for ?
There was a problem hiding this comment.
I do not know if these functions were used at any point in the past. Currently, they are not use anywhere in the code. So, I do not know what they were used for. What do you think about removing them?
There was a problem hiding this comment.
@badhrinathpa, or would you prefer that the unused code stays in the repo? If so, I can add a "nolint:unused" comment?
|
This pull request has been stale for 30 days and will be closed in 5 days. Comment to keep it open. |
|
This pull request has been stale for 30 days and will be closed in 5 days. Comment to keep it open. |
|
This pull request has been stale for 30 days and will be closed in 5 days. Comment to keep it open. |
|
This pull request has been stale for 30 days and will be closed in 5 days. Comment to keep it open. |
|
This pull request has been stale for 30 days and will be closed in 5 days. Comment to keep it open. |
|
@dependabot rebase |
|
This pull request has been stale for 30 days and will be closed in 5 days. Comment to keep it open. |
|
This pull request has been stale for 30 days and will be closed in 5 days. Comment to keep it open. |
This PR addresses a few things related to the GO code in the test directory and includes:
io/ioutilpackagebreakin switch statementreturnstatement, so remove code after panictime.NewTickerinstead oftime.Tick(which leaks the underlying ticker)