-
Notifications
You must be signed in to change notification settings - Fork 41
remove-hardcoded-description #217
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,6 @@ | ||
| kind: fixed | ||
| body: Remove hardcoded description when create/import items | ||
| time: 2026-04-19T10:59:23.659719244Z | ||
| custom: | ||
| Author: aviatco | ||
| AuthorLink: https://github.com/aviatco |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -19,7 +19,6 @@ def test_fabric_data_pipelines_workspace_identity_no_params_success(): | |
| """Test FabricDataPipelines with WorkspaceIdentity credential type when no parameters are required.""" | ||
| # Arrange | ||
| payload = { | ||
| "description": "Created by fab", | ||
| "displayName": "test-connection", | ||
| "connectivityType": "ShareableCloud" | ||
| } | ||
|
|
@@ -63,7 +62,6 @@ def test_connection_with_required_params_missing_failure(): | |
| """Test that connection creation fails when required parameters are missing.""" | ||
| # Arrange | ||
| payload = { | ||
| "description": "Created by fab", | ||
| "displayName": "test-connection", | ||
| "connectivityType": "ShareableCloud" | ||
| } | ||
|
|
@@ -107,7 +105,6 @@ def test_workspace_identity_with_unsupported_params_ignored_success(): | |
| """Test that WorkspaceIdentity ignores unsupported credential parameters with warning.""" | ||
| # Arrange | ||
| payload = { | ||
| "description": "Created by fab", | ||
| "displayName": "test-connection", | ||
| "connectivityType": "ShareableCloud" | ||
| } | ||
|
|
@@ -147,6 +144,83 @@ def test_workspace_identity_with_unsupported_params_ignored_success(): | |
| assert result["credentialDetails"]["credentials"]["credentialType"] == "WorkspaceIdentity" | ||
|
|
||
|
|
||
| def test_connection_payload_does_not_contain_description_by_default_success(): | ||
| """Verify that get_connection_config_from_params never stamps a description | ||
| in the returned payload when the caller does not supply one.""" | ||
| payload = { | ||
| "displayName": "test-connection", | ||
| "connectivityType": "ShareableCloud", | ||
| } | ||
|
|
||
| con_type = "FabricDataPipelines" | ||
| con_type_def = { | ||
| "type": "FabricDataPipelines", | ||
| "creationMethods": [ | ||
| { | ||
| "name": "FabricDataPipelines.Actions", | ||
| "parameters": [], | ||
| } | ||
| ], | ||
| "supportedCredentialTypes": ["WorkspaceIdentity"], | ||
| } | ||
|
|
||
| params = { | ||
| "connectiondetails": { | ||
| "type": "FabricDataPipelines", | ||
| "creationmethod": "FabricDataPipelines.Actions", | ||
| }, | ||
| "credentialdetails": { | ||
| "type": "WorkspaceIdentity", | ||
| }, | ||
| } | ||
|
|
||
| result = get_connection_config_from_params( | ||
| payload, con_type, con_type_def, params) | ||
|
|
||
| assert "description" not in result, ( | ||
| f"Connection payload must not contain 'description' by default, got: {result}" | ||
| ) | ||
|
|
||
|
|
||
| def test_connection_payload_contains_description_when_user_provides_it_success(): | ||
| """Verify that when a user explicitly passes description via params it is | ||
| forwarded into the payload returned by get_connection_config_from_params.""" | ||
| payload = { | ||
| "displayName": "test-connection", | ||
| "connectivityType": "ShareableCloud", | ||
| "description": "My custom description", # user supplied before calling helper | ||
| } | ||
|
|
||
| con_type = "FabricDataPipelines" | ||
| con_type_def = { | ||
| "type": "FabricDataPipelines", | ||
| "creationMethods": [ | ||
| { | ||
| "name": "FabricDataPipelines.Actions", | ||
| "parameters": [], | ||
| } | ||
| ], | ||
| "supportedCredentialTypes": ["WorkspaceIdentity"], | ||
| } | ||
|
|
||
| params = { | ||
| "connectiondetails": { | ||
| "type": "FabricDataPipelines", | ||
| "creationmethod": "FabricDataPipelines.Actions", | ||
| }, | ||
| "credentialdetails": { | ||
| "type": "WorkspaceIdentity", | ||
| }, | ||
| } | ||
|
|
||
| result = get_connection_config_from_params( | ||
| payload, con_type, con_type_def, params) | ||
|
|
||
| assert result.get("description") == "My custom description", ( | ||
| f"Connection payload must preserve a user-supplied description, got: {result}" | ||
| ) | ||
|
Comment on lines
+185
to
+221
|
||
|
|
||
|
|
||
| class TestFindMpeConnection: | ||
| """Test cases for find_mpe_connection function.""" | ||
|
|
||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This test claims the user provides
description“via params”, but it injectsdescriptiondirectly into the inputpayloadinstead. That doesn’t verify the actual CLI path (wheredescriptionwould arrive inargs.params) and will still pass even ifmkdir connectioncontinues to ignoredescription. Move the description intoparams(and/or adjust the docstring) so the test validates param forwarding behavior.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@copilot apply changes based on this feedback