Skip to content

Commit

Permalink
Removes leading whitespace when tailoring BUILD files without heade…
Browse files Browse the repository at this point in the history
…r text (#13394)

Resolves #13351:

Additionally to allowing different names for `tailor`ed BUILD files, new behaviour was added to add copyright headers to BUILD files, but no tests were present to observe behaviour when the default (empty) header text was used. 

This adds such a test, and replaces the default header case with None. Things seem to work better now.

(Cherry-pick of 056c3fd / #13375)

# Rust tests and lints will be skipped. Delete if not intended.
[ci skip-rust]

# Building wheels and fs_util will be skipped. Delete if not intended.
[ci skip-build-wheels]
  • Loading branch information
Christopher Neugebauer authored Oct 28, 2021
1 parent 20d67bf commit 35f2c8f
Show file tree
Hide file tree
Showing 2 changed files with 107 additions and 42 deletions.
10 changes: 5 additions & 5 deletions src/python/pants/core/goals/tailor.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
from abc import ABCMeta
from collections import defaultdict
from dataclasses import dataclass
from typing import Iterable, Mapping, cast
from typing import Iterable, Mapping, Optional, cast

from pants.base.specs import (
AddressSpecs,
Expand Down Expand Up @@ -259,7 +259,7 @@ def register_options(cls, register):
"--build-file-header",
advanced=True,
type=str,
default="",
default=None,
help="A header, e.g., a copyright notice, to add to the content of created BUILD files.",
)

Expand All @@ -285,8 +285,8 @@ def build_file_name(self) -> str:
return cast(str, self.options.build_file_name)

@property
def build_file_header(self) -> str:
return cast(str, self.options.build_file_header)
def build_file_header(self) -> str | None:
return cast(Optional[str], self.options.build_file_header)

@property
def build_file_indent(self) -> str:
Expand Down Expand Up @@ -416,7 +416,7 @@ async def restrict_conflicting_sources(ptgt: PutativeTarget) -> DisjointSourcePu
class EditBuildFilesRequest:
putative_targets: PutativeTargets
name: str
header: str
header: str | None
indent: str


Expand Down
139 changes: 102 additions & 37 deletions src/python/pants/core/goals/tailor_test.py
Original file line number Diff line number Diff line change
@@ -1,8 +1,10 @@
# Copyright 2021 Pants project contributors (see CONTRIBUTORS.md).
# Licensed under the Apache License, Version 2.0 (see LICENSE).
from __future__ import annotations

import textwrap
from dataclasses import dataclass
from textwrap import dedent

import pytest

Expand Down Expand Up @@ -123,19 +125,19 @@ def test_make_content_str() -> None:
],
)
assert (
textwrap.dedent(
"""
fortran_library()
dedent(
"""\
fortran_library()
fortran_tests(
name="tests",
sources=[
"test1.f90",
"test2.f90",
],
)
"""
).lstrip()
fortran_tests(
name="tests",
sources=[
"test1.f90",
"test2.f90",
],
)
"""
)
== content
)

Expand Down Expand Up @@ -285,40 +287,75 @@ def test_edit_build_files(rule_runner: RuleRunner, name: str) -> None:
expected = [
FileContent(
f"src/fortran/baz/{name}.pants",
textwrap.dedent(
"""
Copyright © 2021 FooCorp.
dedent(
"""\
Copyright © 2021 FooCorp.
fortran_library()
fortran_library()
"""
)
.lstrip()
.encode(),
).encode(),
),
FileContent(
f"src/fortran/foo/{name}",
textwrap.dedent(
"""\
fortran_library(sources=["bar1.f90"])
# A comment spread
# over multiple lines.
fortran_library(
name="foo0",
sources=[
"bar2.f90",
"bar3.f90",
],
)
fortran_tests(
name="tests",
life_the_universe_and_everything=42,
)
"""
fortran_library(sources=["bar1.f90"])
).encode(),
),
]
actual = list(contents)
# We do these more laborious asserts instead of just comparing the lists so that
# on a text mismatch we see the actual string diff on the decoded strings.
assert len(expected) == len(actual)
for efc, afc in zip(expected, actual):
assert efc.path == afc.path
assert efc.content.decode() == afc.content.decode()
assert efc.is_executable == afc.is_executable

# A comment spread
# over multiple lines.
fortran_library(
name="foo0",
sources=[
"bar2.f90",
"bar3.f90",
],
)

fortran_tests(
name="tests",
life_the_universe_and_everything=42,
)
"""
)
.lstrip()
.encode(),
def test_edit_build_files_without_header_text(rule_runner: RuleRunner) -> None:
rule_runner.create_dir("src/fortran/baz/BUILD") # NB: A directory, not a file.
req = EditBuildFilesRequest(
PutativeTargets(
[
PutativeTarget.for_target_type(
FortranLibrary, "src/fortran/baz", "baz", ["qux1.f90"]
),
]
),
name="BUILD",
header=None,
indent=" ",
)
edited_build_files = rule_runner.request(EditedBuildFiles, [req])

assert edited_build_files.created_paths == ("src/fortran/baz/BUILD.pants",)

contents = rule_runner.request(DigestContents, [edited_build_files.digest])
expected = [
FileContent(
"src/fortran/baz/BUILD.pants",
dedent(
"""\
fortran_library()
"""
).encode(),
),
]
actual = list(contents)
Expand All @@ -331,6 +368,34 @@ def test_edit_build_files(rule_runner: RuleRunner, name: str) -> None:
assert efc.is_executable == afc.is_executable


@pytest.mark.parametrize("header", [None, "I am some header text"])
def test_build_file_lacks_leading_whitespace(rule_runner: RuleRunner, header: str | None) -> None:
rule_runner.create_dir("src/fortran/baz/BUILD") # NB: A directory, not a file.
req = EditBuildFilesRequest(
PutativeTargets(
[
PutativeTarget.for_target_type(
FortranLibrary, "src/fortran/baz", "baz", ["qux1.f90"]
),
]
),
name="BUILD",
header=header,
indent=" ",
)
edited_build_files = rule_runner.request(EditedBuildFiles, [req])

assert edited_build_files.created_paths == ("src/fortran/baz/BUILD.pants",)

contents = rule_runner.request(DigestContents, [edited_build_files.digest])
actual = list(contents)
# We do these more laborious asserts instead of just comparing the lists so that
# on a text mismatch we see the actual string diff on the decoded strings.
for afc in actual:
content = afc.content.decode()
assert content.lstrip() == content


def test_group_by_dir() -> None:
paths = {
"foo/bar/baz1.ext",
Expand Down

0 comments on commit 35f2c8f

Please sign in to comment.