You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
tableau/server-client-python#1811 fixed six independent bugs in UserItem.CSVImport (issue tableau/server-client-python#1809). Once tabcmd delegates its CSV stack to TSC's CSVImport (planned follow-up), this fixture is the regression guarantee that the delegation preserved each fix.
Behavior
Adds a two-row CSV fixture and one createsiteusers e2e test exercising, in one file:
@property_is_enum(Auth) guard active on the parsed auth_setting.
Test plan
Runs under the same server-admin / site-admin gate as test_users_create_site_users and shares its ordering slot; skips cleanly when the caller lacks admin scope or a credentials file.
Adds a two-row CSV fixture and one createsiteusers e2e test exercising
every UserItem.CSVImport bug fixed in TSC #1811 (issue #1809): AUTH
column readable at 8 columns, mixed-case usernames preserved,
TableauIDWithMFA in the auth allowlist, case-insensitive validation
of role/admin/publisher tokens, and the @property_is_enum guard
active on the parsed auth_setting.
Test runs under the same server-admin / site-admin gate as
test_users_create_site_users and shares its ordering slot; skips
cleanly when the caller lacks admin scope or a credentials file.
Related: #297 (license-name case sensitivity still surfacing after
partial fixes), #434 (Local auth accepted by the CLI but crashes on
TSC call).
The reason will be displayed to describe this comment to others. Learn more.
🟡 Changes recommended
The new e2e test is likely to fail under the command’s default strict CSV validation mode, so it needs adjustment (or coordinated parser/validator changes) before it can be reliably merged.
Get a fresh assessment by requesting another Copilot review.
Pull request overview
Adds an end-to-end regression fixture intended to cover previously fixed TSC.UserItem.CSVImport edge cases by running createsiteusers against a purpose-built two-row CSV file, so tabcmd can later delegate CSV parsing to TSC with confidence.
Changes:
Add a new e2e test that runs createsiteusers using a dedicated “CSVImport bugs” fixture.
Add a new CSV asset containing two rows designed to exercise CSVImport/auth parsing edge cases.
File summaries
File
Description
tests/e2e/online_tests.py
Adds a new ordered e2e test invoking createsiteusers with the new regression CSV fixture.
tests/assets/users_csvimport_bugs_1811.csv
Adds a two-row CSV intended to exercise CSVImport/auth parsing behaviors.
…11 test
`createsiteusers` runs `--complete` by default, which invokes tabcmd's
duplicate strict validator (`UserCommand._validate_user_or_throw`).
That validator rejects `len(line) > 7` and validates license values
case-sensitively -- both of which the CSVImport #1811 fixture is
specifically meant to exercise. Under `--complete`, the CSV never
reached TSC's `UserItem.CSVImport` and the test fell through
tabcmd-side.
`--no-complete` skips tabcmd's validator so the fixture reaches TSC's
parser. Removing `--role Publisher` lets each row's own license field
be validated -- restoring coverage of the case-insensitive-license fix.
Also corrected the docstring: five of the six bugs, since the sixth
(unknown-AUTH rejection) is a negative-path check that doesn't fit a
single happy-path fixture and is already covered by TSC's unit tests.
The reason will be displayed to describe this comment to others. Learn more.
Copilot review overview
🟡 Changes recommended
The new test’s docstring/fixture currently don’t accurately align with what createsiteusers exercises today, so the test won’t cover the behaviors it claims to.
Get a fresh assessment by requesting another Copilot review.
Add mixed-case role and permission tokens to fixture
tests/assets/users_csvimport_bugs_1811.csv:2
The test docstring says this fixture exercises case-insensitive parsing for role/admin/publisher tokens, but the CSV currently keeps the admin/publisher columns lowercase (none,no). Capitalizing/mixing those tokens here would make the test actually cover that behavior.
``--no-complete`` skips tabcmd's duplicate strict validator so the CSV
actually reaches TSC's ``UserItem.CSVImport`` — which is what this test
is meant to exercise. No ``--role`` override, so the role field in each
row is what gets validated (that's the point of the case-insensitivity
check).
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
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.
Motivation
tableau/server-client-python#1811 fixed six independent bugs in
UserItem.CSVImport(issue tableau/server-client-python#1809). Once tabcmd delegates its CSV stack to TSC'sCSVImport(planned follow-up), this fixture is the regression guarantee that the delegation preserved each fix.Behavior
Adds a two-row CSV fixture and one
createsiteuserse2e test exercising, in one file:TableauIDWithMFAaccepted in the auth allowlist,@property_is_enum(Auth)guard active on the parsedauth_setting.Test plan
Runs under the same server-admin / site-admin gate as
test_users_create_site_usersand shares its ordering slot; skips cleanly when the caller lacks admin scope or a credentials file.Related
Localauth accepted by the CLI but crashes on TSC call, related validation-split issue