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

Initial version of the TO API test tool via the Golang TO Client #1604

Merged
merged 40 commits into from
Dec 7, 2017

Conversation

dewrich
Copy link
Contributor

@dewrich dewrich commented Nov 29, 2017

The Traffic Ops API test tool implements a basic Traffic Ops API test suite that utilizes the Golang TO Client. This tool should help with the following issues:

  1. The need for regression tests against the Traffic Ops API
  2. Ensure that the Golang TO Client is being actively used, as well as grow it's
    functionality to be in parity with the Traffic Ops API

High Level Features:

  • Setup from a clean database of the user to access the TO API
  • Teardown of all of the related traffic_ops database tables on startup to ensure test integrity
  • Utilizes the authorization.go module created in traffic_ops/traffic_ops_golang, which can also be used for the next Golang implementations for the TO Golang rewrite effort endponts /login and /users
  • All configuration (config file) options are overridable via command line environment variables, which sets the stage for Dockerization
  • Basic /cdns CRUD implemented (as well as enhancements to the TO Client to support the CRUD operations)
  • NO id centric test fixtures to be found in ./test_cdn.json to simplify maintenance. To load this file, the existing lib/go-tc Golang structs were used to allow for more reuse of the go-tc structs and continual use to ensure their accuracy.

TODOs:

  • Update README.md
  • Make the test_cdn.json a command line flag to allow for multiple test data sets
    and default accordingly

@dewrich dewrich added Traffic Ops API (golang) WIP "Work-in-Progress" - do not merge! (use 'draft' pull requests from now on) labels Nov 29, 2017
@asfgit
Copy link
Contributor

asfgit commented Nov 29, 2017

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/incubator-trafficcontrol-PR/704/
Test PASSed.

@dewrich dewrich requested a review from dangogh November 30, 2017 15:32
@dneuman64
Copy link
Contributor

There is no description provided, so I have to ask, does this replace the golang client integration tests? https://github.com/apache/incubator-trafficcontrol/tree/master/traffic_ops/client_tests/tests/integration

@dewrich
Copy link
Contributor Author

dewrich commented Nov 30, 2017

@dneuman64 Yes, I wanted to separate the TO API client tests from the TO Client itself, so that the TO Client gets exercised as true external clients would. This helps catch any potential functions that need to be exported for TO Golang clients. I do plan on removing the old integration ones once this matures.

@dewrich dewrich added this to In Progress in TO API Golang Rewrite Nov 30, 2017
"fmt"
"net/http"

tc "github.com/apache/incubator-trafficcontrol/lib/go-tc"
Copy link
Member

Choose a reason for hiding this comment

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

This isn't hurting anything, but it doesn't strictly need the alias, Go will automatically import go-tc as tc.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed

@asfgit
Copy link
Contributor

asfgit commented Nov 30, 2017

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/incubator-trafficcontrol-PR/708/
Test PASSed.

@asfgit
Copy link
Contributor

asfgit commented Nov 30, 2017

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/incubator-trafficcontrol-PR/709/
Test PASSed.

Copy link
Member

@dangogh dangogh left a comment

Choose a reason for hiding this comment

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

looks like the vendor directory could use some cleanup, too.. For instance, I don't see the /crypto library used at all...

return cdns, err
var (
API_v2_CDNs = "/api/1.2/cdns"
)
Copy link
Member

Choose a reason for hiding this comment

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

that should probably be a const

Copy link
Contributor Author

Choose a reason for hiding this comment

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

right, I'll fix

if err := json.NewDecoder(resp.Body).Decode(&alerts); err != nil {
return tc.Alerts{}, reqInf, err
}
return alerts, reqInf, nil
Copy link
Member

Choose a reason for hiding this comment

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

Don't need to check for err != nil, just return alerts, reqInf, err.

if err := json.NewDecoder(resp.Body).Decode(&alerts); err != nil {
return tc.Alerts{}, reqInf, err
}
return alerts, reqInf, nil
Copy link
Member

Choose a reason for hiding this comment

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

ditto

@@ -0,0 +1 @@
- Read the fixture .json file for TestCDNs
Copy link
Member

Choose a reason for hiding this comment

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

still needs the Apache license text...

import (
"testing"

log "github.com/apache/incubator-trafficcontrol/lib/go-log"
Copy link
Member

Choose a reason for hiding this comment

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

same comment as @rob05c had elsewhere -- import name is already log, so doesn't need to be specified here


"github.com/kelseyhightower/envconfig"
)

Copy link
Member

Choose a reason for hiding this comment

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

can this be reused from traffic_ops_golang?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If you are referring to using "vendor" code from other components, I have not been able to figure that out yet. But that dependency is only in this tool atm

Copy link
Member

Choose a reason for hiding this comment

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

It can, by putting traffic_ops_golang/config.go in its own package. So, move to traffic_ops_golang/config/config.go, and things like this can import (toconfig "traffic_ops_golang/config")

(I'd name the package toconfig directly if anything besides testing frameworks need it, but that seems unlikely.)

Copy link
Member

Choose a reason for hiding this comment

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

we should address this later...

@@ -0,0 +1,10 @@
language: go
Copy link
Member

Choose a reason for hiding this comment

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

this dir needs to be added to .rat-excludes at the top level of tc or rat will complain...

@asfgit
Copy link
Contributor

asfgit commented Nov 30, 2017

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/incubator-trafficcontrol-PR/710/
Test PASSed.

@asfgit
Copy link
Contributor

asfgit commented Nov 30, 2017

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/incubator-trafficcontrol-PR/711/
Test PASSed.

@asfgit
Copy link
Contributor

asfgit commented Nov 30, 2017

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/incubator-trafficcontrol-PR/712/
Test PASSed.

@asfgit
Copy link
Contributor

asfgit commented Dec 4, 2017

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/incubator-trafficcontrol-PR/715/
Test PASSed.

@asfgit
Copy link
Contributor

asfgit commented Dec 4, 2017

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/incubator-trafficcontrol-PR/719/
Test PASSed.

@asfgit
Copy link
Contributor

asfgit commented Dec 4, 2017

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/incubator-trafficcontrol-PR/724/
Test PASSed.

@asfgit
Copy link
Contributor

asfgit commented Dec 4, 2017

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/incubator-trafficcontrol-PR/725/
Test PASSed.

@asfgit
Copy link
Contributor

asfgit commented Dec 4, 2017

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/incubator-trafficcontrol-PR/726/
Test PASSed.

@dewrich dewrich added this to the 2.2.0 milestone Dec 5, 2017
@dewrich dewrich moved this from In Progress to In Review in TO API Golang Rewrite Dec 5, 2017
@dewrich dewrich removed the WIP "Work-in-Progress" - do not merge! (use 'draft' pull requests from now on) label Dec 6, 2017
@asfgit
Copy link
Contributor

asfgit commented Dec 6, 2017

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/incubator-trafficcontrol-PR/737/
Test PASSed.

@asfgit
Copy link
Contributor

asfgit commented Dec 7, 2017

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/incubator-trafficcontrol-PR/742/
Test PASSed.

@asfgit
Copy link
Contributor

asfgit commented Dec 7, 2017

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/incubator-trafficcontrol-PR/744/
Test FAILed.

@asfgit
Copy link
Contributor

asfgit commented Dec 7, 2017

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/incubator-trafficcontrol-PR/748/
Test PASSed.

@asfgit
Copy link
Contributor

asfgit commented Dec 7, 2017

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/incubator-trafficcontrol-PR/749/
Test PASSed.

@dangogh dangogh merged commit 098cea1 into apache:master Dec 7, 2017
@asfgit
Copy link
Contributor

asfgit commented Dec 7, 2017

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/incubator-trafficcontrol-PR/750/
Test PASSed.

@asfgit
Copy link
Contributor

asfgit commented Dec 7, 2017

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/incubator-trafficcontrol-PR/751/
Test PASSed.

@dewrich dewrich moved this from In Review to Done in TO API Golang Rewrite Feb 6, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

None yet

6 participants