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

testing: Remove old default value from NewTestAgent() calls #7562

Merged
merged 3 commits into from
Apr 1, 2020

Conversation

dnephin
Copy link
Contributor

@dnephin dnephin commented Mar 31, 2020

Follow up to #7546

NewTestAgent was being called with t.Name(), which was removed from name. This PR removes the name and uses the default value.

I also noticed that NewTestAgentWithFields was only starting the agent, not creating a new one, so I renamed that to better reflect the behaviour.

Mostly an automated change with:

git grep -l 'NewTestAgent(t, t.Name(),' | xargs sed -i -e 's/NewTestAgent(t, t.Name(),/NewTestAgent(t,/g'
git grep -l 'StartTestAgent(t, true,' |  xargs sed -i -e 's/StartTestAgent(t, true,/StartTestAgent(t,/g'

Using:

git grep -l 'NewTestAgent(t, t.Name(),' | \
    xargs sed -i -e 's/NewTestAgent(t, t.Name(),/NewTestAgent(t,/g'
@dnephin dnephin changed the title Dnephin/remove tname from name testing: Remove old default value from NewTestAgent() calls Mar 31, 2020
@dnephin dnephin added the theme/internal-cleanup Used to identify tech debt, testing improvements, code refactoring, and non-impactful optimization label Mar 31, 2020
After removing the t.Name() parameter with sed, convert the last few tests which
use a custom name to call NewTestAgentWithFields instead.
This function now only starts the agent.

Using:

git grep -l 'StartTestAgent(t, true,' | \
        xargs sed -i -e 's/StartTestAgent(t, true,/StartTestAgent(t,/g'
@dnephin dnephin force-pushed the dnephin/remove-tname-from-name branch from dd56f91 to e759daa Compare March 31, 2020 21:15
Copy link
Member

@hanshasselberg hanshasselberg left a comment

Choose a reason for hiding this comment

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

LGTM Looking forward to all the improvements! 🎉

@dnephin dnephin merged commit 0d8edc3 into master Apr 1, 2020
@dnephin dnephin deleted the dnephin/remove-tname-from-name branch April 1, 2020 15:48
pierresouchay added a commit to pierresouchay/consul that referenced this pull request Apr 1, 2020
Due to merge hashicorp#7562, upstream does not compile anymore.

Error is:

ERRO Running error: gofmt: analysis skipped: errors in package: [/Users/p.souchay/go/src/github.com/hashicorp/consul/agent/config_endpoint_test.go:188:33: too many arguments]
dnephin added a commit that referenced this pull request Apr 1, 2020
…change_args_in_NewTestAgent

[FIX BUILD] fix build due to merge of #7562
pierresouchay added a commit to pierresouchay/consul that referenced this pull request Apr 1, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
theme/internal-cleanup Used to identify tech debt, testing improvements, code refactoring, and non-impactful optimization
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants