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

Remove dead test #54040

Merged
merged 1 commit into from Jul 29, 2019

Conversation

@waynew
Copy link
Contributor

commented Jul 26, 2019

From what I can tell, this test is no longer relevant. It was put in to
solve issue #2731, where Salt minions were checking DNS even when
--local was passed. Now the code to check the DNS for the master has
grown quite a bit more complex. In salt/minion.py if you simply
force check_dns = True (i.e. remove the local check), this test
will still pass running locally. If this test were effective at testing
whether or not the CLI was going to timeout, it should fail - but it
doesn't.

From what I can gather, there's no longer a good reason to run this
particular test.

What does this PR do?

Fixes #53948 (by removing the affected test)

What issues does this PR fix or reference?

#53948

Previous Behavior

Test intermittently failed, not for any valid reason

New Behavior

Test is gone

Commits signed with GPG?

Yes

@waynew waynew requested a review from dwoz Jul 26, 2019
@dwoz
dwoz approved these changes Jul 29, 2019
From what I can tell, this test is no longer relevant. It was put in to
solve issue #2731, where Salt minions were checking DNS even when
`--local` was passed. Now the code to check the DNS for the master has
grown quite a bit more complex. In [salt/minion.py][1] if you simply
force `check_dns = True` (i.e. remove the `local` check), this test
will still pass running locally. If this test were effective at testing
whether or not the CLI was going to timeout, it should fail - but it
doesn't.

From what I can gather, there's no longer a good reason to run this
particular test.

[1]: https://github.com/saltstack/salt/blob/ffa70adec3ed5a1823976c7f32e37f165a9adb37/salt/minion.py#L138
@waynew waynew force-pushed the waynew:remove-dead-test branch from b850d23 to e9a5a57 Jul 29, 2019
@waynew waynew merged commit 5d3bcd7 into saltstack:2019.2.1 Jul 29, 2019
3 of 24 checks passed
3 of 24 checks passed
ci/py2/amazon2 This commit is being built
Details
ci/py2/centos6 This commit is being built
Details
ci/py2/centos7 This commit is being built
Details
ci/py2/centos7/tcp This commit is being built
Details
ci/py2/debian8 This commit is being built
Details
ci/py2/debian9 This commit is being built
Details
ci/py2/fedora29 This commit is being built
Details
ci/py2/ubuntu1604 This commit is being built
Details
ci/py2/ubuntu1604/tcp This commit is being built
Details
ci/py2/ubuntu1804 This commit is being built
Details
ci/py2/windows2016 This commit is being built
Details
ci/py3/amazon2 This commit is being built
Details
ci/py3/centos7 This commit is being built
Details
ci/py3/centos7/tcp This commit is being built
Details
ci/py3/debian8 This commit is being built
Details
ci/py3/debian9 This commit is being built
Details
ci/py3/fedora29 This commit is being built
Details
ci/py3/ubuntu1604 This commit is being built
Details
ci/py3/ubuntu1604/tcp This commit is being built
Details
ci/py3/ubuntu1804 This commit is being built
Details
ci/py3/windows2016 This commit is being built
Details
WIP Ready for review
Details
ci/docs This commit looks good
Details
ci/lint This commit looks good
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.