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

Skip GCE unit tests - causes test suite to hang #34852

Merged
merged 1 commit into from Jul 21, 2016

Conversation

rallytime
Copy link
Contributor

@rallytime rallytime commented Jul 21, 2016

We need to skip these GCE cloud unit tests because the mocked token in these tests isn't quite right and causes the test suite to hang:

14:23:37 Please Go to the following URL and sign in:
14:23:37 https://accounts.google.com/o/oauth2/auth?scope=https%3A%2F%2Fwww.googleapis.com%2Fauth%2Fcompute+https%3A%2F%2Fwww.googleapis.com%2Fauth%2Fdevstorage.full_control+https%3A%2F%2Fwww.googleapis.com%2Fauth%2Fndev.clouddns.readwrite&state=Libcloud+Request&redirect_uri=urn%3Aietf%3Awg%3Aoauth%3A2.0%3Aoob&response_type=code&client_id=dany%40targaryen.westeros.cloud
14:49:59 Enter Code: Build timed out (after 75 minutes). Marking the build as aborted.
14:49:59 Build was aborted

We are seeing this on the unit.cloud.clouds.gce_test.GCETestCase.test_destroy_call test, specifically.

@tonybaloney These were your tests, so I wanted to ping you. Here's a couple of concerns I have with these tests (and apologies for not catching this sooner). The last three tests appear to be mocked so heavily, that we're not really testing much here. We're basically saying "yeah, since we don't have a real connection, we can't really list available nodes or sizes, so just assert that this is empty." Which misses most of the logic of these functions. The test that is causing the hanging problem, while a little more exciting since we're testing that an error gets raised, is doing the same thing - if you can't get a connection (which we're attempting to mock - perhaps something with libcloud changed that is causing the hang?), then raise an error.

One thing I'd like to point out is that we actually have some integration tests that we run that does check for the successful deletion of a node in the tests/integration/cloud/clouds/gce.py file. Those specific cloud provider tests don't run during the regular/public-facing jenkins test runs, but we do run them behind the scenes.

That all being said, I'm inclined to move the last three tests over to the integration file to make sure those functions are covered in a more robust way, and then delete these unit tests. However, I wanted to get your thoughts/opinion before removing them. However, I'm ok with leaving them in, as long as the mock token problem is addressed. For the time being, I am skipping these so we can get our test suite back on track.

@cachedout cachedout merged commit b3d8143 into saltstack:2016.3 Jul 21, 2016
@rallytime rallytime deleted the skip-gce-tests branch July 21, 2016 17:57
@tonybaloney
Copy link
Contributor

@rallytime a test that does next to nothing is always better than no test :-) I'll improve the tests in a seperate PR. It was used as a way of validating the GCE cloud module in SuSE linux. It uncovered that a few of the salt-cloud modules didn't work in SuSE.

@tonybaloney tonybaloney mentioned this pull request Jul 22, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants