bito-code-review[bot] commented on code in PR #38200:
URL: https://github.com/apache/superset/pull/38200#discussion_r2843845679
##########
superset-extensions-cli/tests/test_templates.py:
##########
@@ -170,42 +181,44 @@ def
test_backend_pyproject_toml_template_renders_correctly(jinja_env, template_c
rendered = template.render(template_context)
# Basic content verification (without full TOML parsing)
- assert "test_extension" in rendered
+ assert "test_org-test_extension" in rendered
assert "0.1.0" in rendered
assert "Apache-2.0" in rendered
# Template Rendering with Different Parameters Tests
@pytest.mark.unit
@pytest.mark.parametrize(
- "extension_id,name,backend_name",
+ "publisher,technical_name,display_name",
Review Comment:
<div>
<div id="suggestion">
<div id="issue"><b>Wrong type for parametrize names</b></div>
<div id="fix">
Change the parametrize names argument from a string to a tuple: change
`"publisher,technical_name,display_name"` to `("publisher", "technical_name",
"display_name")`.
</div>
<details>
<summary>
<b>Code suggestion</b>
</summary>
<blockquote>Check the AI-generated fix before applying</blockquote>
<div id="code">
````suggestion
("publisher", "technical_name", "display_name"),
````
</div>
</details>
</div>
<small><i>Code Review Run #e5c791</i></small>
</div>
---
Should Bito avoid suggestions like this for future reviews? (<a
href=https://alpha.bito.ai/home/ai-agents/review-rules>Manage Rules</a>)
- [ ] Yes, avoid them
##########
superset-extensions-cli/tests/test_name_transformations.py:
##########
@@ -20,381 +20,515 @@
from superset_extensions_cli.exceptions import ExtensionNameError
from superset_extensions_cli.utils import (
generate_extension_names,
+ get_module_federation_name,
kebab_to_camel_case,
kebab_to_snake_case,
name_to_kebab_case,
- to_snake_case, # Keep this for backward compatibility testing only
- validate_extension_name,
+ suggest_technical_name,
+ validate_display_name,
validate_extension_id,
validate_npm_package_name,
+ validate_publisher,
validate_python_package_name,
+ validate_technical_name,
)
-class TestNameTransformations:
- """Test name transformation functions."""
-
- @pytest.mark.parametrize(
- "display_name,expected",
- [
- ("Hello World", "hello-world"),
- ("Data Explorer", "data-explorer"),
- ("My Extension", "my-extension"),
- ("hello-world", "hello-world"), # Already normalized
- ("Hello@World!", "helloworld"), # Special chars removed
- (
- "Data_Explorer",
- "data-explorer",
- ), # Underscores become spaces then hyphens
- ("My Extension", "my-extension"), # Multiple spaces normalized
- (" Hello World ", "hello-world"), # Trimmed
- ("API v2 Client", "api-v2-client"), # Numbers preserved
- ("Simple", "simple"), # Single word
- ],
- )
- def test_name_to_kebab_case(self, display_name, expected):
- """Test direct kebab case conversion from display names."""
- assert name_to_kebab_case(display_name) == expected
-
- @pytest.mark.parametrize(
- "kebab_name,expected",
- [
- ("hello-world", "helloWorld"),
- ("data-explorer", "dataExplorer"),
- ("my-extension", "myExtension"),
- ("api-v2-client", "apiV2Client"),
- ("simple", "simple"), # Single word
- ("chart-tool", "chartTool"),
- ("dashboard-helper", "dashboardHelper"),
- ],
- )
- def test_kebab_to_camel_case(self, kebab_name, expected):
- """Test kebab-case to camelCase conversion."""
- assert kebab_to_camel_case(kebab_name) == expected
-
- @pytest.mark.parametrize(
- "kebab_name,expected",
- [
- ("hello-world", "hello_world"),
- ("data-explorer", "data_explorer"),
- ("my-extension", "my_extension"),
- ("api-v2-client", "api_v2_client"),
- ("simple", "simple"), # Single word
- ("chart-tool", "chart_tool"),
- ("dashboard-helper", "dashboard_helper"),
- ],
- )
- def test_kebab_to_snake_case(self, kebab_name, expected):
- """Test kebab-case to snake_case conversion."""
- assert kebab_to_snake_case(kebab_name) == expected
-
- # Backward compatibility test for remaining legacy function
- @pytest.mark.parametrize(
- "input_name,expected",
- [
- ("hello-world", "hello_world"),
- ("data-explorer", "data_explorer"),
- ("my-extension-name", "my_extension_name"),
- ],
- )
- def test_to_snake_case_legacy(self, input_name, expected):
- """Test legacy kebab-to-snake conversion function."""
- assert to_snake_case(input_name) == expected
-
-
-class TestValidation:
- """Test validation functions."""
-
- @pytest.mark.parametrize(
- "valid_display",
- [
- "Hello World",
- "Data Explorer",
- "My Extension",
- "Simple",
- " Extra Spaces ", # Gets normalized
- ],
- )
- def test_validate_extension_name_valid(self, valid_display):
- """Test valid display names."""
- result = validate_extension_name(valid_display)
- assert result # Should return normalized name
- assert " " not in result # No double spaces
-
- @pytest.mark.parametrize(
- "invalid_display,error_match",
- [
- ("", "cannot be empty"),
- (" ", "cannot be empty"),
- ("@#$%", "must contain at least one letter or number"),
- ],
- )
- def test_validate_extension_name_invalid(self, invalid_display,
error_match):
- """Test invalid extension names."""
- with pytest.raises(ExtensionNameError, match=error_match):
- validate_extension_name(invalid_display)
-
- @pytest.mark.parametrize(
- "valid_id",
- [
- "hello-world",
- "data-explorer",
- "myext",
- "chart123",
- "my-tool-v2",
- "a", # Single character
- "extension-with-many-parts",
- ],
- )
- def test_validate_extension_id_valid(self, valid_id):
- """Test valid extension IDs."""
- # Should not raise exceptions
- validate_extension_id(valid_id)
-
- @pytest.mark.parametrize(
- "invalid_id,error_match",
- [
- ("", "cannot be empty"),
- ("Hello-World", "Use lowercase"),
- ("-hello", "cannot start with hyphens"),
- ("hello-", "cannot end with hyphens"),
- ("hello--world", "consecutive hyphens"),
- ],
- )
- def test_validate_extension_id_invalid(self, invalid_id, error_match):
- """Test invalid extension IDs."""
- with pytest.raises(ExtensionNameError, match=error_match):
- validate_extension_id(invalid_id)
-
- @pytest.mark.parametrize(
- "valid_package",
- [
- "hello_world",
- "data_explorer",
- "myext",
- "test123",
- "package_with_many_parts",
- ],
- )
- def test_validate_python_package_name_valid(self, valid_package):
- """Test valid Python package names."""
- # Should not raise exceptions
- validate_python_package_name(valid_package)
-
- @pytest.mark.parametrize(
- "keyword",
- [
- "class",
- "import",
- "def",
- "return",
- "if",
- "else",
- "for",
- "while",
- "try",
- "except",
- "finally",
- "with",
- "as",
- "lambda",
- "yield",
- "False",
- "None",
- "True",
- ],
- )
- def test_validate_python_package_name_keywords(self, keyword):
- """Test that Python reserved keywords are rejected."""
- with pytest.raises(
- ExtensionNameError, match="Package name cannot start with Python
keyword"
- ):
- validate_python_package_name(keyword)
-
- @pytest.mark.parametrize(
- "invalid_package",
- [
- "hello-world", # Hyphens not allowed in Python identifiers
- ],
- )
- def test_validate_python_package_name_invalid(self, invalid_package):
- """Test invalid Python package names."""
- with pytest.raises(ExtensionNameError, match="not a valid Python
package"):
- validate_python_package_name(invalid_package)
-
- @pytest.mark.parametrize(
- "valid_npm",
- [
- "hello-world",
+# Name transformation tests
+
+
[email protected](
+ "display_name,expected",
Review Comment:
<div>
<div id="suggestion">
<div id="issue"><b>Parametrize decorator expects tuple not string</b></div>
<div id="fix">
The first argument to `@pytest.mark.parametrize` should be a tuple of
parameter names, not a string. Change `"display_name,expected"` to
`("display_name", "expected")`. This issue appears 10+ times throughout the
file.
</div>
<details>
<summary>
<b>Code suggestion</b>
</summary>
<blockquote>Check the AI-generated fix before applying</blockquote>
<div id="code">
````suggestion
@pytest.mark.parametrize(
("display_name", "expected"),
````
</div>
</details>
</div>
<small><i>Code Review Run #e5c791</i></small>
</div>
---
Should Bito avoid suggestions like this for future reviews? (<a
href=https://alpha.bito.ai/home/ai-agents/review-rules>Manage Rules</a>)
- [ ] Yes, avoid them
##########
superset-extensions-cli/tests/test_cli_init.py:
##########
@@ -133,41 +134,43 @@ def
test_init_creates_extension_with_neither_frontend_nor_backend(
@pytest.mark.cli
def test_init_accepts_any_display_name(cli_runner, isolated_filesystem):
"""Test that init accepts any display name and generates proper ID."""
- cli_input = "My Awesome Extension!\n\n0.1.0\nApache-2.0\ny\ny\n"
+ cli_input = "My Awesome Extension!\n\ntest-org\n0.1.0\nApache-2.0\ny\ny\n"
result = cli_runner.invoke(app, ["init"], input=cli_input)
assert result.exit_code == 0, f"Should accept display name:
{result.output}"
- assert Path("my-awesome-extension").exists(), (
- "Directory for generated ID should be created"
+ assert Path("test-org.my-awesome-extension").exists(), (
+ "Directory for generated composite ID should be created"
)
@pytest.mark.cli
def test_init_accepts_mixed_alphanumeric_name(cli_runner, isolated_filesystem):
"""Test that init accepts mixed alphanumeric display names."""
- cli_input = "Tool 123\n\n0.1.0\nApache-2.0\ny\ny\n"
+ cli_input = "Tool 123\n\ntest-org\n0.1.0\nApache-2.0\ny\ny\n"
result = cli_runner.invoke(app, ["init"], input=cli_input)
assert result.exit_code == 0, (
f"Mixed alphanumeric display name should be valid: {result.output}"
)
- assert Path("tool-123").exists(), "Directory for 'tool-123' should be
created"
+ assert Path("test-org.tool-123").exists(), (
+ "Directory for 'test-org.tool-123' should be created"
+ )
@pytest.mark.cli
@pytest.mark.parametrize(
"display_name,expected_id",
Review Comment:
<div>
<div id="suggestion">
<div id="issue"><b>Wrong type for parametrize argument</b></div>
<div id="fix">
Change `parametrize` argument from string to tuple format: use
`("display_name,expected_id", [...])` instead of `"display_name,expected_id"`.
</div>
<details>
<summary>
<b>Code suggestion</b>
</summary>
<blockquote>Check the AI-generated fix before applying</blockquote>
<div id="code">
````suggestion
("display_name", "expected_id"),
````
</div>
</details>
</div>
<small><i>Code Review Run #e5c791</i></small>
</div>
---
Should Bito avoid suggestions like this for future reviews? (<a
href=https://alpha.bito.ai/home/ai-agents/review-rules>Manage Rules</a>)
- [ ] Yes, avoid them
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
