Skip to content
This repository has been archived by the owner on Jan 11, 2023. It is now read-only.

Unit tests and minor code cleanup for GetTenantID. #3467

Merged
merged 1 commit into from
Jul 13, 2018

Conversation

tariq1890
Copy link
Contributor

@tariq1890 tariq1890 commented Jul 12, 2018

What this PR does / why we need it: Trying out net/http/test for writing unit tests for GetTenantID

Special notes for your reviewer:

If applicable:

  • documentation
  • unit tests
  • tested backward compatibility (ie. deploy with previous version, upgrade with this branch)

Release note:

@tariq1890 tariq1890 force-pushed the get_tenantid_test branch from 7d803c5 to 6c2355a Compare July 12, 2018 04:57
"github.com/pkg/errors"
log "github.com/sirupsen/logrus"
)

// GetTenantID figures out the AAD tenant ID of the subscription by making an
// unauthenticated request to the Get Subscription Details endpoint and parses
// the value from WWW-Authenticate header.
func GetTenantID(env azure.Environment, subscriptionID string) (string, error) {
func GetTenantID(resourceManagerEndpoint string, subscriptionID string) (string, error) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

With this change, we are able to do away with an unnecessary import :)

Copy link
Contributor

@CecileRobertMichon CecileRobertMichon left a comment

Choose a reason for hiding this comment

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

lgtm

@codecov
Copy link

codecov bot commented Jul 13, 2018

Codecov Report

Merging #3467 into master will increase coverage by 0.09%.
The diff coverage is 66.66%.

@@            Coverage Diff             @@
##           master    #3467      +/-   ##
==========================================
+ Coverage   55.83%   55.93%   +0.09%     
==========================================
  Files         105      105              
  Lines       15884    15883       -1     
==========================================
+ Hits         8869     8884      +15     
+ Misses       6270     6253      -17     
- Partials      745      746       +1

@CecileRobertMichon CecileRobertMichon merged commit cc53e77 into Azure:master Jul 13, 2018
julienstroheker pushed a commit to julienstroheker/acs-engine that referenced this pull request Jul 16, 2018
kkmsft pushed a commit to kkmsft/acs-engine that referenced this pull request Jul 20, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants