From f41af69b6ef5d8ddc768f9905cfd5b9242405b00 Mon Sep 17 00:00:00 2001 From: Sachin Holla <51310506+sachinholla@users.noreply.github.com> Date: Fri, 10 Jul 2020 05:12:59 +0530 Subject: [PATCH] Cleanup translib and cvl go test cases (#13) - Why I did it There are few issues in cvl and translib go test code. - How I did it 1) Fixed compilation is translib test code. Temporarily disabled ACL test codes from translib test since there are too many errors in it. Moved all common utility code in ACL test file into a new common file (app_utils_test.go). 2) Fixed the failures in cvl test code. 3) Added a new app module api_tests_app.go which provides dummy implementations of all app interface APIs for paths starting with /api-tests:. Errors can be simulated by including token /error/ in the path. This can be used by translib test code as well as translib client test code (REST or telemetry) to verify end-to-end of all cases without depending on actual app modules or database state. - How to verify it Run translib and cvl go test cases - Description for the changelog Cleanup translib and cvl go test cases --- cvl/cvl_test.go | 37 ++---- cvl/internal/util/util.go | 2 +- translib/Makefile | 2 +- translib/acl_app_test.go | 73 +---------- translib/api_tests_app.go | 188 ++++++++++++++++++++++++++++ translib/app_utils_test.go | 124 ++++++++++++++++++ translib/transformer/transformer.go | 14 ++- 7 files changed, 341 insertions(+), 99 deletions(-) create mode 100644 translib/api_tests_app.go create mode 100644 translib/app_utils_test.go diff --git a/cvl/cvl_test.go b/cvl/cvl_test.go index afd3e80efc91..3731311b63ed 100644 --- a/cvl/cvl_test.go +++ b/cvl/cvl_test.go @@ -20,7 +20,6 @@ package cvl_test import ( - "github.com/Azure/sonic-mgmt-common/cvl" "encoding/json" "fmt" "github.com/go-redis/redis" @@ -31,6 +30,7 @@ import ( "syscall" "testing" "runtime" + "github.com/Azure/sonic-mgmt-common/cvl" . "github.com/Azure/sonic-mgmt-common/cvl/internal/util" "github.com/Azure/sonic-mgmt-common/cvl/internal/yparser" ) @@ -2862,35 +2862,18 @@ func TestValidateEditConfig_Delete_Entry_Then_Dep_Leafref_Positive(t *testing.T) } func TestBadSchema(t *testing.T) { - env := os.Environ() - env[0] = env[0] + " " - - if _, err := os.Stat("/usr/sbin/schema"); os.IsNotExist(err) { - //Corrupt some schema file - exec.Command("/bin/sh", "-c", "/bin/cp testdata/schema/sonic-port.yin testdata/schema/sonic-port.yin.bad" + - " && /bin/sed -i '1 a ' testdata/schema/sonic-port.yin.bad").Output() - - //Parse bad schema file - if module, _ := yparser.ParseSchemaFile("testdata/schema/sonic-port.yin.bad"); module != nil { //should fail - t.Errorf("Bad schema parsing should fail.") - } - - //Revert to - exec.Command("/bin/sh", "-c", "/bin/rm testdata/schema/sonic-port.yin.bad").Output() - } else { - //Corrupt some schema file - exec.Command("/bin/sh", "-c", "/bin/cp /usr/sbin/schema/sonic-port.yin /usr/sbin/schema/sonic-port.yin.bad" + - " && /bin/sed -i '1 a ' /usr/sbin/schema/sonic-port.yin.bad").Output() + badSchema, err := ioutil.TempFile("", "sonic-test-*.yin") + if err != nil { + t.Fatalf("could not create temp file") + } - //Parse bad schema file - if module, _ := yparser.ParseSchemaFile("/usr/sbin/schema/sonic-port.yin.bad"); module != nil { //should fail - t.Errorf("Bad schema parsing should fail.") - } + // write incomplete module data to temporary schema file + badSchema.WriteString("") + badSchema.Close() - //Revert to - exec.Command("/bin/sh", "-c", "/bin/rm /usr/sbin/schema/sonic-port.yin.bad").Output() + if module, _ := yparser.ParseSchemaFile(badSchema.Name()); module != nil { //should fail + t.Errorf("Bad schema parsing should fail.") } - } diff --git a/cvl/internal/util/util.go b/cvl/internal/util/util.go index 7404500d0469..57d99e10f1b9 100644 --- a/cvl/internal/util/util.go +++ b/cvl/internal/util/util.go @@ -32,7 +32,7 @@ import ( log "github.com/golang/glog" ) -var CVL_SCHEMA string = "schema/" +var CVL_SCHEMA string = "/usr/sbin/schema/" var CVL_CFG_FILE string = "/usr/sbin/cvl_cfg.json" //package init function diff --git a/translib/Makefile b/translib/Makefile index 4acbd3140602..360b573543a0 100644 --- a/translib/Makefile +++ b/translib/Makefile @@ -37,7 +37,7 @@ $(TRANSLIB_PKG): $(TRANSLIB_MAIN_SRCS) $(YGOT_BINDS) $(GO) build -mod=vendor -gcflags="all=-N -l" -v -o $@ ../translib $(TRANSLIB_TEST_BIN): $(TRANSLIB_MAIN_SRCS) $(TRANSLIB_TEST_SRCS) $(YGOT_BINDS) - $(GO) test -mod=vendor -cover -coverpkg=../translib,../translib/tlerr -c ../translib -o $@ + $(GO) test -mod=vendor -tags test -cover -coverpkg=../translib,../translib/tlerr -c ../translib -o $@ $(TRANSL_DB_TEST_BIN) : $(TRANSL_DB_ALL_SRCS) $(GO) test -mod=vendor -cover -c ../translib/db -o $@ diff --git a/translib/acl_app_test.go b/translib/acl_app_test.go index 729c288b5a6b..3442d7b5b92a 100644 --- a/translib/acl_app_test.go +++ b/translib/acl_app_test.go @@ -17,35 +17,23 @@ // // //////////////////////////////////////////////////////////////////////////////// +// +build !test + package translib import ( "errors" "fmt" - "os" "reflect" "strings" "testing" + db "github.com/Azure/sonic-mgmt-common/translib/db" ) func init() { fmt.Println("+++++ Init acl_app_test +++++") -} - -func TestMain(m *testing.M) { - if err := clearAclDataFromDb(); err != nil { - os.Exit(-1) - } - fmt.Println("+++++ Removed All Acl Data from Db +++++") - - ret := m.Run() - - if err := clearAclDataFromDb(); err != nil { - os.Exit(-1) - } - - os.Exit(ret) + addCleanupFunc("ACL", clearAclDataFromDb) } // This will test GET on /openconfig-acl:acl @@ -344,48 +332,6 @@ func Test_AclApp_NegativeTests(t *testing.T) { t.Run("Verify_Top_Level_Delete", processGetRequest(topLevelUrl, emptyJson, false)) } -func processGetRequest(url string, expectedRespJson string, errorCase bool) func(*testing.T) { - return func(t *testing.T) { - response, err := Get(GetRequest{url}) - if err != nil && !errorCase { - t.Errorf("Error %v received for Url: %s", err, url) - } - - respJson := response.Payload - if string(respJson) != expectedRespJson { - t.Errorf("Response for Url: %s received is not expected:\n%s", url, string(respJson)) - } - } -} - -func processSetRequest(url string, jsonPayload string, oper string, errorCase bool) func(*testing.T) { - return func(t *testing.T) { - var err error - switch oper { - case "POST": - _, err = Create(SetRequest{Path: url, Payload: []byte(jsonPayload)}) - case "PATCH": - _, err = Update(SetRequest{Path: url, Payload: []byte(jsonPayload)}) - case "PUT": - _, err = Replace(SetRequest{Path: url, Payload: []byte(jsonPayload)}) - default: - t.Errorf("Operation not supported") - } - if err != nil && !errorCase { - t.Errorf("Error %v received for Url: %s", err, url) - } - } -} - -func processDeleteRequest(url string) func(*testing.T) { - return func(t *testing.T) { - _, err := Delete(SetRequest{Path: url}) - if err != nil { - t.Errorf("Error %v received for Url: %s", err, url) - } - } -} - // THis will delete ACL table and Rules Table from DB func clearAclDataFromDb() error { var err error @@ -408,17 +354,6 @@ func clearAclDataFromDb() error { return err } -func getConfigDb() *db.DB { - configDb, _ := db.NewDB(db.Options{ - DBNo: db.ConfigDB, - InitIndicator: "CONFIG_DB_INITIALIZED", - TableNameSeparator: "|", - KeySeparator: "|", - }) - - return configDb -} - func Test_AclApp_Subscribe(t *testing.T) { app := new(AclApp) diff --git a/translib/api_tests_app.go b/translib/api_tests_app.go new file mode 100644 index 000000000000..463a4b35c4ff --- /dev/null +++ b/translib/api_tests_app.go @@ -0,0 +1,188 @@ +//////////////////////////////////////////////////////////////////////////////// +// // +// Copyright 2020 Broadcom. The term Broadcom refers to Broadcom Inc. and/or // +// its subsidiaries. // +// // +// Licensed under the Apache License, Version 2.0 (the "License"); // +// you may not use this file except in compliance with the License. // +// You may obtain a copy of the License at // +// // +// http://www.apache.org/licenses/LICENSE-2.0 // +// // +// Unless required by applicable law or agreed to in writing, software // +// distributed under the License is distributed on an "AS IS" BASIS, // +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. // +// See the License for the specific language governing permissions and // +// limitations under the License. // +// // +//////////////////////////////////////////////////////////////////////////////// + +// +build test + +package translib + +import ( + "encoding/json" + "reflect" + "strings" + + "github.com/Azure/sonic-mgmt-common/translib/db" + "github.com/Azure/sonic-mgmt-common/translib/tlerr" + "github.com/golang/glog" +) + +// apiTests is an app module for testing translib APIs. +// Implements dummy handlers for paths starting with "/api-tests:". +// Returns error if path contains "/error/"; see getError function. +type apiTests struct { + path string + body []byte + appOptions + + echoMsg string + echoErr string +} + +func init() { + err := register("/api-tests:", + &appInfo{ + appType: reflect.TypeOf(apiTests{}), + isNative: true, + tablesToWatch: nil}) + + if err != nil { + glog.Fatalf("Failed to register ApiTest app; %v", err) + } +} + +func (app *apiTests) initialize(inp appData) { + app.path = inp.path + app.body = inp.payload + app.appOptions = inp.appOptions +} + +func (app *apiTests) translateCreate(d *db.DB) ([]db.WatchKeys, error) { + return nil, app.translatePath() +} + +func (app *apiTests) translateUpdate(d *db.DB) ([]db.WatchKeys, error) { + return nil, app.translatePath() +} + +func (app *apiTests) translateReplace(d *db.DB) ([]db.WatchKeys, error) { + return nil, app.translatePath() +} + +func (app *apiTests) translateDelete(d *db.DB) ([]db.WatchKeys, error) { + return nil, app.translatePath() +} + +func (app *apiTests) translateGet(dbs [db.MaxDB]*db.DB) error { + return app.translatePath() +} + +func (app *apiTests) translateAction(dbs [db.MaxDB]*db.DB) error { + var req struct { + Input struct { + Message string `json:"message"` + ErrType string `json:"error-type"` + } `json:"api-tests:input"` + } + + err := json.Unmarshal(app.body, &req) + if err != nil { + glog.Errorf("Failed to parse rpc input; err=%v", err) + return tlerr.InvalidArgs("Invalid rpc input") + } + + app.echoMsg = req.Input.Message + app.echoErr = req.Input.ErrType + + return nil +} + +func (app *apiTests) translateSubscribe(dbs [db.MaxDB]*db.DB, path string) (*notificationOpts, *notificationInfo, error) { + return nil, nil, nil +} + +func (app *apiTests) processCreate(d *db.DB) (SetResponse, error) { + return app.processSet() +} + +func (app *apiTests) processUpdate(d *db.DB) (SetResponse, error) { + return app.processSet() +} + +func (app *apiTests) processReplace(d *db.DB) (SetResponse, error) { + return app.processSet() +} + +func (app *apiTests) processDelete(d *db.DB) (SetResponse, error) { + return app.processSet() +} + +func (app *apiTests) processGet(dbs [db.MaxDB]*db.DB) (GetResponse, error) { + var gr GetResponse + err := app.getError() + if err != nil { + return gr, err + } + + resp := make(map[string]interface{}) + resp["message"] = app.echoMsg + resp["path"] = app.path + resp["depth"] = app.depth + + gr.Payload, err = json.Marshal(&resp) + return gr, err +} + +func (app *apiTests) processAction(dbs [db.MaxDB]*db.DB) (ActionResponse, error) { + var ar ActionResponse + + err := app.getError() + if err == nil { + var respData struct { + Output struct { + Message string `json:"message"` + } `json:"api-tests:output"` + } + + respData.Output.Message = app.echoMsg + ar.Payload, err = json.Marshal(&respData) + } + + return ar, err +} + +func (app *apiTests) translatePath() error { + app.echoMsg = "Hello, world!" + k := strings.Index(app.path, "error/") + if k >= 0 { + app.echoErr = app.path[k+6:] + } + return nil +} + +func (app *apiTests) processSet() (SetResponse, error) { + var sr SetResponse + err := app.getError() + return sr, err +} + +func (app *apiTests) getError() error { + switch strings.ToLower(app.echoErr) { + case "invalid-args", "invalidargs": + return tlerr.InvalidArgs(app.echoMsg) + case "exists": + return tlerr.AlreadyExists(app.echoMsg) + case "not-found", "notfound": + return tlerr.NotFound(app.echoMsg) + case "not-supported", "notsupported", "unsupported": + return tlerr.NotSupported(app.echoMsg) + case "", "no", "none", "false": + return nil + default: + return tlerr.New(app.echoMsg) + } +} diff --git a/translib/app_utils_test.go b/translib/app_utils_test.go new file mode 100644 index 000000000000..adf536dbe67a --- /dev/null +++ b/translib/app_utils_test.go @@ -0,0 +1,124 @@ +//////////////////////////////////////////////////////////////////////////////// +// // +// Copyright 2019 Broadcom. The term Broadcom refers to Broadcom Inc. and/or // +// its subsidiaries. // +// // +// Licensed under the Apache License, Version 2.0 (the "License"); // +// you may not use this file except in compliance with the License. // +// You may obtain a copy of the License at // +// // +// http://www.apache.org/licenses/LICENSE-2.0 // +// // +// Unless required by applicable law or agreed to in writing, software // +// distributed under the License is distributed on an "AS IS" BASIS, // +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. // +// See the License for the specific language governing permissions and // +// limitations under the License. // +// // +//////////////////////////////////////////////////////////////////////////////// + +package translib + +import ( + "os" + "testing" + + "github.com/Azure/sonic-mgmt-common/translib/db" + "github.com/golang/glog" +) + +/* This file includes common utilities for app module test code. */ + +// CleanupFunc is the callback function used for cleaning config_db +// entries before and after tests. Should be registered via addCleanupFunc. +type CleanupFunc func() error + +var cleanupFuncs map[string]CleanupFunc + +// addCleanupFunc registers a cleanup function. Should be called from +// init() of individual test files if they need pre/post cleanup. +func addCleanupFunc(name string, f CleanupFunc) { + if cleanupFuncs == nil { + cleanupFuncs = map[string]CleanupFunc{} + } + + cleanupFuncs[name] = f +} + +func TestMain(m *testing.M) { + // cleanup before tests + for name, f := range cleanupFuncs { + if err := f(); err != nil { + glog.Errorf("%s cleanup failed; err=%v", name, err) + os.Exit(-1) + } else { + glog.Infof("Cleanup %s before tests", name) + } + } + + ret := m.Run() + + // cleanup after tests + for name, f := range cleanupFuncs { + if err := f(); err != nil { + glog.Warningf("Cleanup %s failed; err=%v", name, err) + } else { + glog.Infof("Cleanup %s after tests", name) + } + } + + os.Exit(ret) +} + +func processGetRequest(url string, expectedRespJson string, errorCase bool) func(*testing.T) { + return func(t *testing.T) { + response, err := Get(GetRequest{Path: url}) + if err != nil && !errorCase { + t.Fatalf("Error %v received for Url: %s", err, url) + } + + respJson := response.Payload + if string(respJson) != expectedRespJson { + t.Fatalf("Response for Url: %s received is not expected:\n%s", url, string(respJson)) + } + } +} + +func processSetRequest(url string, jsonPayload string, oper string, errorCase bool) func(*testing.T) { + return func(t *testing.T) { + var err error + switch oper { + case "POST": + _, err = Create(SetRequest{Path: url, Payload: []byte(jsonPayload)}) + case "PATCH": + _, err = Update(SetRequest{Path: url, Payload: []byte(jsonPayload)}) + case "PUT": + _, err = Replace(SetRequest{Path: url, Payload: []byte(jsonPayload)}) + default: + t.Errorf("Operation not supported") + } + if err != nil && !errorCase { + t.Errorf("Error %v received for Url: %s", err, url) + } + } +} + +func processDeleteRequest(url string) func(*testing.T) { + return func(t *testing.T) { + _, err := Delete(SetRequest{Path: url}) + if err != nil { + t.Errorf("Error %v received for Url: %s", err, url) + } + } +} + +func getConfigDb() *db.DB { + configDb, _ := db.NewDB(db.Options{ + DBNo: db.ConfigDB, + InitIndicator: "CONFIG_DB_INITIALIZED", + TableNameSeparator: "|", + KeySeparator: "|", + }) + + return configDb +} diff --git a/translib/transformer/transformer.go b/translib/transformer/transformer.go index 05eb21a2923b..2a06c2dde68d 100644 --- a/translib/transformer/transformer.go +++ b/translib/transformer/transformer.go @@ -29,7 +29,7 @@ import ( "io/ioutil" ) -const YangPath = "/usr/models/yang/" // OpenConfig-*.yang and sonic yang models path +var YangPath = "/usr/models/yang/" // OpenConfig-*.yang and sonic yang models path var entries = map[string]*yang.Entry{} @@ -85,6 +85,7 @@ func getDefaultModelsList () ([]string) { } func init() { + initYangModelsPath() yangFiles := []string{} ocList := getOcModelsList() yangFiles = getDefaultModelsList() @@ -96,6 +97,17 @@ func init() { } } +func initYangModelsPath() { + if path, ok := os.LookupEnv("YANG_MODELS_PATH"); ok { + if !strings.HasSuffix(path, "/") { + path += "/" + } + YangPath = path + } + + fmt.Println("Yang models path:", YangPath) +} + func loadYangModules(files ...string) error { var err error