101 test tableauauth model.py by talvalin · Pull Request #134 · tableau/server-client-python

talvalin · 2017-01-23T09:37:33Z

I thought I branched from development this time, but I'm still getting the analytics files in my pull request. How do I fix this? :(

…mplfied test to check for just the TypeError and ignore the exception text

graysonarts · 2017-01-23T17:14:32Z

🚀 lgtm. Sadly the analytics script commits is cruft that will stick around for a little while.

talvalin · 2017-01-24T08:25:58Z

On a related note, could you explain how I can create the test coverage stats in order to update the stats in #101?

graysonarts · 2017-01-24T17:53:53Z

@talvalin if you pip install coverage, then you will have a coverage command line tool and then you would do something like this:

coverage run --source=tableauserverclient setup.py test
coverage report

This should give you an output that looks like this:

Name                                                          Stmts   Miss  Cover
---------------------------------------------------------------------------------
tableauserverclient/__init__.py                                   5      0   100%
tableauserverclient/datetime_helpers.py                          18      2    89%
tableauserverclient/models/__init__.py                           15      0   100%
tableauserverclient/models/connection_credentials.py             17      8    53%
tableauserverclient/models/connection_item.py                    38      0   100%
tableauserverclient/models/datasource_item.py                   105      5    95%
tableauserverclient/models/exceptions.py                          2      0   100%
tableauserverclient/models/fileupload_item.py                    17     10    41%
tableauserverclient/models/group_item.py                         39      0   100%
tableauserverclient/models/interval_item.py                     112      9    92%
tableauserverclient/models/pagination_item.py                    22      0   100%
tableauserverclient/models/project_item.py                       59      1    98%
tableauserverclient/models/property_decorators.py                78     16    79%
tableauserverclient/models/schedule_item.py                     146      5    97%
tableauserverclient/models/server_info_item.py                   21      0   100%
tableauserverclient/models/site_item.py                         147     14    90%
tableauserverclient/models/tableau_auth.py                       18      9    50%
tableauserverclient/models/tag_item.py                           13      0   100%
tableauserverclient/models/user_item.py                         112      4    96%
tableauserverclient/models/view_item.py                          56      6    89%
tableauserverclient/models/workbook_item.py                     147      8    95%
tableauserverclient/namespace.py                                  1      0   100%
tableauserverclient/server/__init__.py                            9      0   100%
tableauserverclient/server/endpoint/__init__.py                  12      0   100%
tableauserverclient/server/endpoint/auth_endpoint.py             34      3    91%
tableauserverclient/server/endpoint/datasources_endpoint.py     102     22    78%
tableauserverclient/server/endpoint/endpoint.py                  56     10    82%
tableauserverclient/server/endpoint/exceptions.py                20      0   100%
tableauserverclient/server/endpoint/fileuploads_endpoint.py      46     31    33%
tableauserverclient/server/endpoint/groups_endpoint.py           78      4    95%
tableauserverclient/server/endpoint/projects_endpoint.py         40      0   100%
tableauserverclient/server/endpoint/schedules_endpoint.py        46      8    83%
tableauserverclient/server/endpoint/server_info_endpoint.py      16      0   100%
tableauserverclient/server/endpoint/sites_endpoint.py            59      4    93%
tableauserverclient/server/endpoint/users_endpoint.py            60      1    98%
tableauserverclient/server/endpoint/views_endpoint.py            23      0   100%
tableauserverclient/server/endpoint/workbooks_endpoint.py       139     13    91%
tableauserverclient/server/exceptions.py                          2      0   100%
tableauserverclient/server/filter.py                             19      3    84%
tableauserverclient/server/pager.py                              25      2    92%
tableauserverclient/server/request_factory.py                   278     46    83%
tableauserverclient/server/request_options.py                    45      3    93%
tableauserverclient/server/server.py                             83      6    93%
tableauserverclient/server/sort.py                                6      3    50%
---------------------------------------------------------------------------------
TOTAL                                                          2386    256    89%

t8y8

Minor changes

t8y8 · 2017-01-25T07:16:42Z

test/test_tableauauth_model.py

+
+
+class TableauAuthModelTests(unittest.TestCase):
+    def test_username_password_required(self):


Just assertRaises the cm object doesn't get used so no need.

Uh, if I remove the cm object, the TypeError gets thrown without being handled and so winds up as an error in the test.

self.assertRaises(TypeError, TSC.TableauAuth())

What am I doing wrong here?

Don't remove it, just don't name it.

with self.assertRaises(TypeError): TSC.TableauAuth()

The as context_manager wasn't necessary because you didn't use the object.
The context manager itself is the way to go

Ah. That makes sense. Cool.

t8y8 · 2017-01-25T07:17:19Z

test/test_tableauauth_model.py

+
+    def test_site_arg_raises_warning(self):
+        with warnings.catch_warnings(record=True) as w:
+            # Cause all warnings to always be triggered.


Comment is unnecessary given the name of the option in the code :)

t8y8 · 2017-01-25T07:18:08Z

test/test_tableauauth_model.py

+            # Cause all warnings to always be triggered.
+            warnings.simplefilter("always")
+
+            tableau_auth = TSC.TableauAuth('user',


That's not the right kind of site id.

This is the site name in the URL just like tabcmd.

t8y8 · 2017-01-25T07:19:56Z

test/test_tableauauth_model.py

+                                           site='dad65087-b08b-4603-af4e-2887b8aafc67')
+
+            assert len(w) == 1
+            assert issubclass(w[-1].category, DeprecationWarning)


Use asserttrue or assert equals? The bare asserts aren't the right style for unittest, that's more of a pytest thing

Will fix, but apparently testing the warning type is proving fun via the unittest asserts.

talvalin · 2017-01-26T07:01:48Z

So I updated following your comments, but I'm finding it hard to test the warnings inside the getter and setter. I tried explicitly calling them in a test, but the coverage report maintains that the lines inside each function are not being called. :(

talvalin · 2017-01-26T08:59:39Z

Coverage report for models\tableau_auth.py says:

Name                                                          Stmts   Miss  Cover
---------------------------------------------------------------------------------
tableauserverclient\models\tableau_auth.py                       18      6    67%

Better than 50%, but still sucking slightly.

t8y8 · 2017-01-26T19:25:53Z

@talvalin I say go ahead and remove the deprecation tests for now -- they'll go away to the tests aren't long term useful.

Focus on covering all the remaining lines (if they aren't already by your PR, I haven't had a chance to test locally)

talvalin · 2017-01-28T03:38:22Z

@RussTheAerialist @t8y8 - So any thoughts on how I can test the getters and setters, or should I just ignore those?

t8y8 · 2017-01-31T18:02:31Z

Don't worry about it for now, no reason to hold this improvement up!

t8y8 · 2017-02-01T19:48:26Z

test/test_tableauauth_model.py

+    def setUp(self):
+        self.auth = TSC.TableauAuth('user',
+                                    'password',
+                                    site_id='dad65087-b08b-4603-af4e-2887b8aafc67',


Wrong site id type still.

This is the thing in the url after /t/ or /site/ NOT a luid.

Just make it the string site1

talvalin · 2017-02-02T11:19:24Z

I realise that you said to take out the warning check, but without that test the coverage stays at 50% rather than 67% so I left it in.

t8y8 · 2017-02-02T17:14:08Z

🚀 some test coverage is better than none.

Let's get this in (it's hackathon!) clear the PRs!

@talvalin

Improve coverage in the auth model tests. Original patch by @talvalin

jdomingu and others added 4 commits January 23, 2017 12:28

Add analytics script

2318634

First attempt at tests for tableau_auth

edd692d

Forgot that assertRegex isn't available in 2.7

257d28b

TypeError exception text is different between 2.8 and 3.x, so just si…

40b2312

…mplfied test to check for just the TypeError and ignore the exception text

t8y8 suggested changes Jan 25, 2017

View reviewed changes

Updated following comments from t8y8

8d9baa4

t8y8 reviewed Feb 1, 2017

View reviewed changes

Updated site_id in SetUp

45110a6

t8y8 approved these changes Feb 2, 2017

View reviewed changes

t8y8 merged commit 8d54ac8 into tableau:development Feb 2, 2017

talvalin deleted the 101-test_tableauauth_model.py branch February 16, 2017 03:14

graysonarts pushed a commit that referenced this pull request Mar 23, 2017

101 test tableauauth model.py (#134)

fbe9b54

Improve coverage in the auth model tests. Original patch by @talvalin



		class TableauAuthModelTests(unittest.TestCase):
		def test_username_password_required(self):

Conversation

talvalin commented Jan 23, 2017

Uh oh!

graysonarts commented Jan 23, 2017

Uh oh!

talvalin commented Jan 24, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

graysonarts commented Jan 24, 2017

Uh oh!

t8y8 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

t8y8 Jan 25, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

talvalin commented Jan 26, 2017

Uh oh!

talvalin commented Jan 26, 2017

Uh oh!

t8y8 commented Jan 26, 2017

Uh oh!

talvalin commented Jan 28, 2017

Uh oh!

t8y8 commented Jan 31, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

talvalin commented Feb 2, 2017

Uh oh!

t8y8 commented Feb 2, 2017

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

talvalin commented Jan 24, 2017 •

edited

Loading

t8y8 Jan 25, 2017 •

edited

Loading