test(format): cover sqlmesh format on BigQuery header lists

Add a Context-level test that formats a BigQuery model with `tags`,
`ignored_rules`, `grain` and `partitioned_by`, then reloads it. Trim
the header-list unit tests and drop the postgres/databricks cases,
which passed with or without the fix.

Signed-off-by: mday-io <mdaytn@gmail.com>

Author: mday-io

tests/core/test_dialect.py

@@ -620,20 +620,8 @@ def test_format_model_expressions_macro_property_comments_preserved_with_dialect
 @pytest.mark.parametrize("dialect", ["bigquery", "duckdb", "snowflake"])
 @pytest.mark.parametrize("prop_name", ["tags", "ignored_rules"])
 def test_format_model_expressions_list_property_array_literal(dialect: str, prop_name: str):
-    """Dialect-agnostic header properties that hold a list (`tags`, `ignored_rules`) must
-    render as a bracketed list literal (`[a, b]`) on dialects that spell arrays that way,
-    not the base generator's `ARRAY(a, b)`.
-
-    On BigQuery, `ARRAY(` is parsed as a subquery constructor, so a multi-element
-    `ARRAY('C1', 'c2')` fails to reparse with `Required keyword: 'value' missing for
-    Property`. This previously affected any dialect using this generator, since these
-    properties render generically (the `dialect=None` path) regardless of the model's
-    own dialect. Only bigquery/duckdb/snowflake-like dialects are covered here;
-    dialects whose own array syntax is not brackets (postgres' `ARRAY[...]`,
-    databricks' `ARRAY(...)`) or that reuse `[`/`]` for identifier quoting (tsql,
-    sqlite, ...) are covered by
-    `test_format_model_expressions_list_property_dialects_without_bracket_arrays`.
-    """
+    """List-valued header properties render as `[a, b]`, not `ARRAY(a, b)`, which
+    BigQuery parses as a subquery and fails to load."""
     source = f"""MODEL (
   name a.b,
   dialect {dialect},
@@ -642,14 +630,9 @@ def test_format_model_expressions_list_property_array_literal(dialect: str, prop
 SELECT 1 AS x"""
 
     formatted = format_model_expressions(parse(source, default_dialect=dialect), dialect=dialect)
-
     assert f"{prop_name} ['C1', 'c2']" in formatted
 
-    # Reparses cleanly with the model's own dialect.
-    reparsed = parse(formatted, default_dialect=dialect)
-
-    # Idempotent: formatting an already-formatted model is a no-op.
-    twice = format_model_expressions(reparsed, dialect=dialect)
+    twice = format_model_expressions(parse(formatted, default_dialect=dialect), dialect=dialect)
     assert formatted == twice
 
     model = load_sql_based_model(parse(formatted, default_dialect=dialect), dialect=dialect)
@@ -660,37 +643,20 @@ def test_format_model_expressions_list_property_array_literal(dialect: str, prop
 
 
 def test_format_model_expressions_array_property_no_dialect_unchanged():
-    """Regression guard: with no model dialect, list-valued header properties must keep
-    rendering through the base generator (`ARRAY(...)`), exactly as pinned by
-    `test_format_model_expressions`. The `[...]` rewrite only applies once a model
-    dialect is present (gated on `meta_dialect`)."""
     formatted = format_model_expressions(
         parse("MODEL (name a.b, tags ['C1', 'c2']); SELECT 1 AS x")
     )
 
     assert "tags ARRAY('C1', 'c2')" in formatted
 
 
-@pytest.mark.parametrize("dialect", ["tsql", "sqlite", "postgres", "databricks"])
+@pytest.mark.parametrize("dialect", ["tsql", "sqlite"])
 @pytest.mark.parametrize("prop_name", ["tags", "ignored_rules"])
-def test_format_model_expressions_list_property_dialects_without_bracket_arrays(
+def test_format_model_expressions_list_property_bracket_identifier_dialects(
     dialect: str, prop_name: str
 ):
-    """Regression: dialects whose own generator does not spell an `exp.Array` as
-    `[a, b]` must NOT get the bracket-list rewrite from
-    `test_format_model_expressions_list_property_array_literal`, and must keep the
-    generic `ARRAY(...)` form.
-
-    This matters most for tsql and sqlite (also true of tableau, exasol, fabric), which
-    reuse `[`/`]` for identifier quoting: `['a', 'b']` is not an array literal in their
-    grammar at all, so rewriting `tags` or `ignored_rules` to that form reparses as a
-    single bracket-quoted identifier, silently collapsing two values into one and
-    corrupting the tag/rule names -- even though this exact source formatted correctly
-    on `main` before bracket rendering was introduced. postgres (`ARRAY[...]`) and
-    databricks (`ARRAY(...)`) are not corrupted by the bracket form, but should still
-    keep rendering with their own generator's spelling rather than a generic bracket
-    literal that is not how either dialect writes arrays.
-    """
+    """These dialects quote identifiers with `[...]`, so a bracketed list would reload
+    as a single identifier. List properties must keep the `ARRAY(...)` form."""
     source = f"""MODEL (
   name a.b,
   dialect {dialect},
@@ -699,13 +665,9 @@ def test_format_model_expressions_list_property_dialects_without_bracket_arrays(
 SELECT 1 AS x"""
 
     formatted = format_model_expressions(parse(source, default_dialect=dialect), dialect=dialect)
+    assert f"{prop_name} ARRAY('C1', 'c2')" in formatted
 
-    prop_line = formatted.split(f"{prop_name} ")[1].split("\n")[0]
-    assert prop_line.startswith("ARRAY")
-    assert "[" not in prop_line
-
-    reparsed = parse(formatted, default_dialect=dialect)
-    twice = format_model_expressions(reparsed, dialect=dialect)
+    twice = format_model_expressions(parse(formatted, default_dialect=dialect), dialect=dialect)
     assert formatted == twice
 
     model = load_sql_based_model(parse(formatted, default_dialect=dialect), dialect=dialect)
@@ -717,12 +679,8 @@ def test_format_model_expressions_list_property_dialects_without_bracket_arrays(
 
 @pytest.mark.parametrize("dialect", ["bigquery", "duckdb", "snowflake", "postgres"])
 def test_format_model_expressions_grain_alias_render_policy(dialect: str):
-    """`grain` is renamed to `grains` in `ModelMeta._pre_root_validator`, not via a
-    Pydantic alias, so `_meta_render_policy` must special-case it to inherit `grains`'
-    render policy (warehouse SQL). Otherwise a multi-column `grain [id, id2]` falls back
-    to the generic, dialect-agnostic path and (via the base generator) becomes
-    `ARRAY(id, id2)`, which fails to reparse on BigQuery.
-    """
+    """`grain` is renamed to `grains` before validation, so it must share the
+    `grains` render policy instead of falling back to `ARRAY(...)`."""
     source = f"""MODEL (
   name a.b,
   dialect {dialect},
@@ -731,29 +689,18 @@ def test_format_model_expressions_grain_alias_render_policy(dialect: str):
 SELECT 1 AS x, 2 AS id, 3 AS id2"""
 
     formatted = format_model_expressions(parse(source, default_dialect=dialect), dialect=dialect)
-
-    # Rendered with the model's own dialect (e.g. postgres' native `ARRAY[...]`), never
-    # the base generator's `ARRAY(id, id2)`, which fails to reparse on BigQuery.
     assert "ARRAY(id, id2)" not in formatted
 
     twice = format_model_expressions(parse(formatted, default_dialect=dialect), dialect=dialect)
     assert formatted == twice
 
-    # `grain [id, id2]` parses to a single composite grain wrapping both columns
-    # (independent of this fix); what matters here is that it survives a dialect-
-    # specific round trip rather than being flattened to the generic `ARRAY(...)`.
     model = load_sql_based_model(parse(formatted, default_dialect=dialect), dialect=dialect)
-    assert len(model.grains) == 1
     assert {c.name for c in model.grains[0].find_all(exp.Column)} == {"id", "id2"}
 
 
 def test_format_model_expressions_table_properties_alias_render_policy():
-    """`table_properties` is the deprecated alias for `physical_properties`, renamed in
-    `ModelMeta._pre_root_validator`, not via a Pydantic alias. It must inherit
-    `physical_properties`' render policy (warehouse SQL) so dialect-specific values
-    inside it, such as tsql's `DATETIME2`, are not flattened to the generic generator's
-    `TIMESTAMP` spelling.
-    """
+    """`table_properties` is renamed to `physical_properties` before validation, so it
+    must keep dialect-specific types such as tsql's `DATETIME2`."""
     formatted = format_model_expressions(
         parse(
             """

tests/core/test_format.py

@@ -161,3 +161,37 @@ def test_format_without_state_load(tmp_path: pathlib.Path, mocker: MockerFixture
     context = Context(paths=tmp_path, config=Config(project="local_only"), load_state=False)
     context.format(check=True)
     mock.assert_not_called()
+
+
+def test_format_bigquery_header_list_properties(tmp_path: pathlib.Path):
+    # A BigQuery model must survive `sqlmesh format` and still load: list-valued header
+    # properties were rewritten to `ARRAY(...)`, which BigQuery parses as a subquery.
+    model_file = create_temp_file(
+        tmp_path,
+        pathlib.Path("models/model.sql"),
+        """MODEL (
+  name test.model,
+  kind INCREMENTAL_BY_TIME_RANGE (time_column ds),
+  tags ['C1', 'c2'],
+  ignored_rules ['noselectstar', 'ambiguousorinvalidcolumn'],
+  grain [id],
+  partitioned_by DATE_TRUNC(ds, MONTH)
+);
+SELECT 1 AS id, CURRENT_DATE() AS ds""",
+    )
+    config = Config(model_defaults=ModelDefaultsConfig(dialect="bigquery"))
+
+    Context(paths=tmp_path, config=config).format()
+
+    formatted = model_file.read_text(encoding="utf-8")
+    assert "tags ['C1', 'c2']" in formatted
+    assert "ignored_rules ['noselectstar', 'ambiguousorinvalidcolumn']" in formatted
+    assert "grain [id]" in formatted
+    assert "partitioned_by DATE_TRUNC(ds, MONTH)" in formatted
+
+    context = Context(paths=tmp_path, config=config)
+    assert context.format(check=True)
+    model = context.get_model("test.model")
+    assert model.tags == ["C1", "c2"]
+    assert model.ignored_rules == {"noselectstar", "ambiguousorinvalidcolumn"}
+    assert [p.sql("bigquery") for p in model.partitioned_by] == ["DATE_TRUNC(`ds`, MONTH)"]