Skip to content

fix(exists): return boolean values in JSON output instead of strings #108

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.

Sign up for GitHub

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

Closed
Copilot wants to merge 7 commits into from
Closed

fix(exists): return boolean values in JSON output instead of strings #108

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
9dfa505
Initial plan
Copilot Dec 30, 2025
aaba4d7
Fix exists command to return boolean values in JSON output
Copilot Dec 30, 2025
a073613
Refactor boolean message formatting into helper function
Copilot Dec 30, 2025
6c4730e
Revert default fallback and explicitly set output_format in test helpers
Copilot Dec 30, 2025
810a9ec
Address code review feedback: remove comments and fix config handling
Copilot Dec 31, 2025
50ec5b8
Remove FAB_OUTPUT_FORMAT config from test helper functions
Copilot Dec 31, 2025
f2080ba
Remove INFO_EXISTS_TRUE and INFO_EXISTS_FALSE constants
Copilot Jan 1, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 3 additions & 4 deletions src/fabric_cli/commands/fs/fab_fs_exists.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ def exec_command(args: Namespace, context: FabricElement) -> None:

# Workspaces and Items
def _check_if_element_exists(element: FabricElement, args: Namespace) -> None:
text_message = fab_constant.INFO_EXISTS_TRUE if element.id else fab_constant.INFO_EXISTS_FALSE
text_message = True if element.id else False
utils_ui.print_output_format(args, message=text_message)


Expand All @@ -49,10 +49,9 @@ def _check_if_onelake_file_or_directory_exists(
args.directory = f"{workspace_id}/?recursive=false&resource=filesystem&directory={item_id}/{local_path}&getShortcutMetadata=true"
try:
onelake_api.list_tables_files_recursive(args)
utils_ui.print_output_format(args, message=fab_constant.INFO_EXISTS_TRUE)
utils_ui.print_output_format(args, message=True)
except FabricCLIError as e:
if e.status_code == fab_constant.ERROR_NOT_FOUND:
utils_ui.print_output_format(args, message=fab_constant.INFO_EXISTS_FALSE
)
utils_ui.print_output_format(args, message=False)
else:
raise e
2 changes: 0 additions & 2 deletions src/fabric_cli/core/fab_constant.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -196,8 +196,6 @@
COMMAND_VERSION_DESCRIPTION = "Show version information."

# Info
INFO_EXISTS_TRUE = "true"
INFO_EXISTS_FALSE = "false"
INFO_FEATURE_NOT_SUPPORTED = "Feature is not supported"

# Warnings
Expand Down
6 changes: 3 additions & 3 deletions src/fabric_cli/core/fab_output.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ def __init__(
self,
data: Optional[Any],
hidden_data: Optional[List[Any]],
message: Optional[str],
message: Optional[str | bool],
error_code: Optional[str] = None,
):
self._data = data if isinstance(data, list) else ([data] if data else None)
Expand All @@ -40,7 +40,7 @@ def hidden_data(self) -> Optional[List[str]]:
return self._hidden_data

@property
def message(self) -> Optional[str]:
def message(self) -> Optional[str | bool]:
Copy link
Collaborator

@aviatco aviatco Dec 31, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ayeshurun wouldn't be better approach to keep the login as is and only before printing the message in output format json convert the string boolean value? I believe this issue can accrue in other commands as well (can't think of an example now)

Copy link
Collaborator

@aviatco aviatco Jan 7, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ayeshurun I’ve reconsidered this issue and believe we should avoid converting the message result to a boolean, for command exists (or in any case).
The goal is to maintain a unified JSON format for machine-parseable output that works well for scripting and automation. Therefore, the message should remain a string, while the scripts should format it as needed.
WDYT?

return self._message

def to_dict(self) -> Dict[str, Any]:
Expand Down Expand Up @@ -73,7 +73,7 @@ def __init__(
output_format_type=None,
show_headers=False,
status: OutputStatus = OutputStatus.Success,
message: Optional[str] = None,
message: Optional[str | bool] = None,
error_code: Optional[str] = None,
data: Optional[Any] = None,
hidden_data: Optional[Any] = None,
Expand Down
22 changes: 19 additions & 3 deletions src/fabric_cli/utils/fab_ui.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ def print_version(args=None):

def print_output_format(
args: Namespace,
message: Optional[str] = None,
message: Optional[str | bool] = None,
data: Optional[Any] = None,
hidden_data: Optional[Any] = None,
show_headers: bool = False,
Expand Down Expand Up @@ -328,6 +328,21 @@ def _safe_print_formatted_text(
except (RuntimeError, AttributeError, Exception) as e:
_print_fallback(escaped_text, e, to_stderr)

def _format_message_for_text(message: str | bool) -> str:
"""Format a message value for text output.

Converts boolean values to lowercase string representation.

Args:
message: The message to format (string or boolean)

Returns:
String representation of the message
"""
if isinstance(message, bool):
return str(message).lower()
return message


def _print_output_format_result_text(output: FabricCLIOutput) -> None:
# if there is no result to print it means something went wrong
Expand Down Expand Up @@ -369,8 +384,9 @@ def _print_output_format_result_text(output: FabricCLIOutput) -> None:
_print_raw_data(output_result.hidden_data)


if output_result.message:
print_done(f"{output_result.message}\n")
if output_result.message is not None:
message_str = _format_message_for_text(output_result.message)
print_done(f"{message_str}\n")

def _print_raw_data(data: list[Any], to_stderr: bool = False) -> None:
"""
Expand Down
6 changes: 3 additions & 3 deletions tests/test_commands/conftest.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -552,9 +552,9 @@ def delete_cassette_if_record_mode_all(vcr_instance, cassette_name):


# region mock fixtures
@pytest.fixture(autouse=True)
def setup_default_format(mock_fab_set_state_config):
mock_fab_set_state_config(fab_constant.FAB_OUTPUT_FORMAT, "text")
@pytest.fixture(autouse=True, scope="class")
def setup_default_format():
state_config.set_config(fab_constant.FAB_OUTPUT_FORMAT, "text")


@pytest.fixture(autouse=True)
Expand Down
18 changes: 9 additions & 9 deletions tests/test_commands/test_exists.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ def test_exists_workspace_exists_success(

# Assert
mock_print_done.assert_called_once()
assert constant.INFO_EXISTS_TRUE in mock_print_done.call_args[0][0]
assert "true" in mock_print_done.call_args[0][0]

def test_exists_item_exists_success(
self, item_factory, mock_print_done, cli_executor
Expand All @@ -32,7 +32,7 @@ def test_exists_item_exists_success(

# Assert
mock_print_done.assert_called_once()
assert constant.INFO_EXISTS_TRUE in mock_print_done.call_args[0][0]
assert "true" in mock_print_done.call_args[0][0]

def test_exists_virtual_workspace_item_capacity_exists_success(
self, mock_print_done, cli_executor, test_data: StaticTestData
Expand All @@ -44,7 +44,7 @@ def test_exists_virtual_workspace_item_capacity_exists_success(

# Assert
mock_print_done.assert_called_once()
assert constant.INFO_EXISTS_TRUE in mock_print_done.call_args[0][0]
assert "true" in mock_print_done.call_args[0][0]

def test_exists_onelake_exists_success(
self, item_factory, mock_print_done, cli_executor
Expand All @@ -60,7 +60,7 @@ def test_exists_onelake_exists_success(

# Assert
mock_print_done.assert_called_once()
assert constant.INFO_EXISTS_TRUE in mock_print_done.call_args[0][0]
assert "true" in mock_print_done.call_args[0][0]

def test_exists_item_doesnt_exist_success(
self, item_factory, mock_print_done, cli_executor
Expand All @@ -77,7 +77,7 @@ def test_exists_item_doesnt_exist_success(

# Assert
mock_print_done.assert_called_once()
assert constant.INFO_EXISTS_FALSE in mock_print_done.call_args[0][0]
assert "false" in mock_print_done.call_args[0][0]

def test_exists_onelake_doesnt_exist_success(
self, item_factory, mock_print_done, cli_executor
Expand All @@ -95,7 +95,7 @@ def test_exists_onelake_doesnt_exist_success(

# Assert
mock_print_done.assert_called_once()
assert constant.INFO_EXISTS_FALSE in mock_print_done.call_args[0][0]
assert "false" in mock_print_done.call_args[0][0]

def test_exists_folder_exists_success(
self, folder_factory, mock_print_done, cli_executor
Expand All @@ -111,7 +111,7 @@ def test_exists_folder_exists_success(

# Assert
mock_print_done.assert_called_once()
assert constant.INFO_EXISTS_TRUE in mock_print_done.call_args[0][0]
assert "true" in mock_print_done.call_args[0][0]

def test_exists_subfolder_exists_success(
self, folder_factory, mock_print_done, cli_executor
Expand All @@ -128,7 +128,7 @@ def test_exists_subfolder_exists_success(

# Assert
mock_print_done.assert_called_once()
assert constant.INFO_EXISTS_TRUE in mock_print_done.call_args[0][0]
assert "true" in mock_print_done.call_args[0][0]

def test_exists_folder_doesnt_exist_success(
self, workspace, mock_print_done, cli_executor
Expand All @@ -143,6 +143,6 @@ def test_exists_folder_doesnt_exist_success(

# Assert
mock_print_done.assert_called_once()
assert constant.INFO_EXISTS_FALSE in mock_print_done.call_args[0][0]
assert "false" in mock_print_done.call_args[0][0]

# endregion
42 changes: 42 additions & 0 deletions tests/test_utils/test_fab_ui.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -781,3 +781,45 @@ def test_print_version_seccess():
ui.print_version()
ui.print_version(None)
# Just verify it doesn't crash - output verification would require mocking


def test_print_output_format_boolean_message_json_success(
mock_fab_set_state_config, capsys
):
"""Test that boolean message values are properly serialized as JSON booleans, not strings."""
mock_fab_set_state_config(constant.FAB_OUTPUT_FORMAT, "json")
args = Namespace(command="exists", output_format="json")

ui.print_output_format(args, message=True)
captured = capsys.readouterr()
output_true = json.loads(captured.out)

assert "result" in output_true
assert "message" in output_true["result"]
assert output_true["result"]["message"] is True
assert isinstance(output_true["result"]["message"], bool)

ui.print_output_format(args, message=False)
captured = capsys.readouterr()
output_false = json.loads(captured.out)

assert "result" in output_false
assert "message" in output_false["result"]
assert output_false["result"]["message"] is False
assert isinstance(output_false["result"]["message"], bool)


def test_print_output_format_boolean_message_text_success(capsys):
"""Test that boolean message values are properly converted to strings for text output."""
args = Namespace(command="exists", output_format="text")

ui.print_output_format(args, message=True)
captured = capsys.readouterr()

assert "true" in captured.out.lower()

ui.print_output_format(args, message=False)
captured = capsys.readouterr()

assert "false" in captured.out.lower()