test(e2e): add e2e tests for workbooks, datasources, views, pagination, jobs, tagging, projects, favorites, metadata, site-admin - #1823
Open
jacalata wants to merge 9 commits into
Open
test(e2e): add e2e tests for workbooks, datasources, views, pagination, jobs, tagging, projects, favorites, metadata, site-admin#1823jacalata wants to merge 9 commits into
jacalata wants to merge 9 commits into
Conversation
Adds integration tests covering: - Workbooks CRUD (create, publish, get, update, download, delete) - Datasources CRUD (publish, get, update connections, refresh, delete) - Views export (PDF, PNG, CSV, Excel) - Tag quoting (spaces and commas in tag names) - Projects get_by_path - Pagination (Pager and QuerySet, order_by, pagesize) - Jobs and async operations (trigger refresh, poll until complete) - Projects admin CRUD and permissions (SiteAdmin) - Users and groups (SiteAdmin) - Workbook permissions (grant, verify, revoke) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Move default_project, project_id, and server_admin into conftest.py so individual test files don't each inline the same TABLEAU_PROJECT env var lookup. Saves ~130 lines of boilerplate across 5 files. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- tagging: guard finally delete_tags with try/except (tag already deleted by test body) - jobs: skip on 403180 when workbook has no extract and refresh is disallowed - workbooks: improve filter assertion error message; use QuerySet filter instead of get() - conftest: add TABLEAU_VERIFY_SSL env var to skip SSL verification for on-prem servers Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…rkbook asset Restores content from old jac/e2e-tests branch (commit cf59ca4): - test_favorites.py: workbook/view/datasource favorites - test_metadata.py: GraphQL metadata API queries - test_site_admin.py: schedules, webhooks, connection update, data freshness, workbook move, user/group management (requires e2e_admin marker) - assets/WorkbookWithExtract.twbx: binary asset for extract refresh tests - test_jobs.py: add extract_workbook fixture and test_extract_workbook_refresh_completes - test_workbooks_crud.py: add test_workbook_populate_preview_image Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…urce tests SampleDS.tds is a live connection to postgres91.test.tsi.lan which Tableau Server tries to validate on publish, causing the fixture to fail if the host is unreachable. WorldIndicators.tdsx is an extract-based datasource that publishes without a connection check. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2 tasks
Contributor
|
Claude code report: Code ReviewReviewed the diff via Blocking: will fail CI black check
Suggestions (non-blocking)
Other notes
🤖 Generated with Claude Code |
Contributor
|
@jacalata overall this looks good. Take a look at the issues Claude noted to see if any are worth fixing/improving. |
- Reformat single-line `if` in test_views_export.py's a_view fixture so black --check passes CI (was blocking meta-checks.yml). - Use conftest.default_project in test_workbook_permissions.workbook_and_user, eliminating 10 lines of duplicated project-lookup logic. - Replace bare `except Exception: pass` in teardown blocks with warnings.warn(...) for consistency with test_pagination.py's workbook cleanup. - Use bare Path for SAMPLE_DATASOURCE calls across test_favorites.py and test_jobs.py (was inconsistently wrapped in str()). - Register the e2e_admin marker in pyproject.toml so it matches conftest.py's pytest_configure. All from bcantoni's review comment on #1823. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Contributor
Author
|
Thanks — fixes applied in 3a4fe13:
Skipped Claude's suggestion to also touch |
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Adds a comprehensive end-to-end (E2E) pytest suite that exercises Tableau Server Client (TSC) APIs against a real Tableau server, including a separate set of SiteAdmin-only tests.
Changes:
- Introduces multiple new E2E test modules covering CRUD, permissions, exports, jobs, pagination, favorites, metadata, and admin operations.
- Adds
e2e_adminmarker plus aserver_adminauthenticated fixture and shared project fixtures for consistent publishing targets. - Expands tagging tests to include datasource tagging scenarios and additional space/comma edge cases.
Reviewed changes
Copilot reviewed 15 out of 17 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| test_e2e/conftest.py | Adds admin-auth session fixture, shared project selection, and SSL verify configuration for E2E runs. |
| test_e2e/test_workbooks_crud.py | New workbook CRUD E2E coverage (publish/get/update/populate/download/delete). |
| test_e2e/test_workbook_permissions.py | New workbook/view permissions E2E coverage using SiteAdmin credentials. |
| test_e2e/test_views_export.py | New view export E2E coverage (PDF/PNG/CSV). |
| test_e2e/test_users_groups.py | New SiteAdmin E2E coverage for user/group lifecycle and membership operations. |
| test_e2e/test_tagging.py | Updates tagging tests; adds datasource tagging and more tag edge-case coverage. |
| test_e2e/test_site_admin.py | New SiteAdmin E2E coverage for schedules, webhooks, connection updates, freshness policy, project moves, and groups. |
| test_e2e/test_projects_get_by_path.py | New E2E/admin split tests for projects.get_by_path() behavior. |
| test_e2e/test_projects_admin.py | New SiteAdmin project CRUD + default permissions E2E coverage. |
| test_e2e/test_pagination.py | New E2E coverage for TSC.Pager and QuerySet pagination behavior. |
| test_e2e/test_metadata.py | New E2E smoke tests for Metadata GraphQL queries. |
| test_e2e/test_jobs.py | New E2E coverage for refresh jobs and wait_for_job() completion paths. |
| test_e2e/test_favorites.py | New E2E coverage for favorites on workbooks/views/datasources. |
| test_e2e/test_datasources_crud.py | New datasource CRUD E2E coverage (publish/get/update/connections/tags/download/delete). |
| pyproject.toml | Registers the new e2e_admin marker. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+81
to
+85
| def test_workbook_download_returns_nonempty_bytes(server, published_workbook): | ||
| """download() writes a non-empty file into a BytesIO buffer.""" | ||
| buffer = io.BytesIO() | ||
| server.workbooks.download(published_workbook.id, filepath=buffer) | ||
| assert buffer.tell() > 0 |
Comment on lines
+73
to
+79
| finally: | ||
| # Clean up: build the delete rule directly to avoid masking the original exception | ||
| delete_rule = TSC.PermissionsRule( | ||
| grantee, | ||
| {TSC.Permission.Capability.Read: TSC.Permission.Mode.Allow}, | ||
| ) | ||
| server_admin.workbooks.delete_permission(workbook, delete_rule) |
Comment on lines
+30
to
+31
| http_options = {"verify": os.environ.get("TABLEAU_VERIFY_SSL", "true").lower() != "false"} | ||
| server = TSC.Server(url, use_server_version=True, http_options=http_options) |
Comment on lines
+32
to
+35
| new_user = TSC.UserItem("tsc-e2e-user", TSC.UserItem.Roles.Unlicensed) | ||
| created = None | ||
| try: | ||
| created = server_admin.users.add(new_user) |
Comment on lines
+54
to
+57
| def test_favorites_view(server, workbook): | ||
| """A view can be added to and removed from favorites.""" | ||
| server.workbooks.populate_views(workbook) | ||
| view = workbook.views[0] |
Comment on lines
+35
to
+41
| schedule = TSC.ScheduleItem( | ||
| _name("tsc-e2e-schedule"), | ||
| 50, | ||
| TSC.ScheduleItem.Type.Extract, | ||
| TSC.ScheduleItem.ExecutionOrder.Parallel, | ||
| interval, | ||
| ) |
Copilot review findings on #1823. (1) test_users_groups.py: every user/group name is now suffixed with a uuid via a _unique(prefix) helper. Fixed names caused 409 conflicts when two runs executed against the same site in parallel, or when a prior run crashed before its finally block removed the resource. (2) test_workbook_permissions.py: wrap the delete_permission call in test_update_permissions_appears_on_populate's finally block in try/except, matching the sibling test_delete_permission_removes_rule. Unwrapped cleanup can raise (e.g. rule already gone, transient server error) and mask the original assertion failure with a confusing "delete_permission raised 500" instead of the actual test failure. No production code touched.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
conftest.py; addsserver_admin,default_project,project_idsession-scoped fixtures ande2e_adminmarkerTest plan
pytest test_e2e/ -v -n 0 -m e2eagainst a Tableau server withTABLEAU_SERVER,TABLEAU_TOKEN_NAME,TABLEAU_TOKEN,TABLEAU_SITE,TABLEAU_PROJECT,TABLEAU_VERIFY_SSLset — 41 passed, 1 known flake (test_datasources_getname-filter timing), 3 skipped pending Add Projects.get_by_path() for hierarchy-aware project lookup #1804 merge (test_projects_get_by_path)pytest test_e2e/test_site_admin.py -v -n 0 -m e2e_adminwith additionalTABLEAU_SITEADMIN_TOKEN_NAME/TABLEAU_SITEADMIN_TOKENenv vars — skipped, test server does not support user creation via REST APItest/— CI passing🤖 Generated with Claude Code