Skip to content
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

chore: Fix inconsistent black version in precommit #2314

Merged
merged 6 commits into from
Oct 22, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
14 changes: 8 additions & 6 deletions .pre-commit-config.yaml
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
repos:
- repo: https://github.com/python/black
rev: 19.3b0
hooks:
- id: black
language_version: python3.7
exclude_types: ['markdown', 'ini', 'toml']
- repo: local
Copy link
Contributor

Choose a reason for hiding this comment

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

could you explain a bit what the new changes are?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

hooks:
- id: black
name: black
entry: black
language: system
require_serial: true
types: [python]
27 changes: 24 additions & 3 deletions DEVELOPMENT_GUIDE.md
Original file line number Diff line number Diff line change
Expand Up @@ -48,11 +48,32 @@ easily setup multiple Python versions.
### 2. Install Additional Tooling
#### Black
We format our code using [Black](https://github.com/python/black) and verify the source code is black compliant
in Appveyor during PRs. You can find installation instructions on [Black's docs](https://black.readthedocs.io/en/stable/installation_and_usage.html).
Install version 19.10b0 as this is what is currently used in the CI/CD pipeline.
in Appveyor during PRs. Black will be installed automatically with `make init`.

After installing, you can run our formatting through our Makefile by `make black-format` or integrating Black directly in your favorite IDE (instructions
After installing, you can run our formatting through our Makefile by `make black` or integrating Black directly in your favorite IDE (instructions
can be found [here](https://black.readthedocs.io/en/stable/editor_integration.html))

##### (workaround) Integrating Black directly in your favorite IDE
Since black is installed in virtualenv, when you follow [this intruction](https://black.readthedocs.io/en/stable/editor_integration.html), `which black` might give you this

```bash
(samcli37) $ where black
/Users/<username>/.pyenv/shims/black
```

However, IDEs such PyChaim (using FileWatcher) will have a hard time invoking `/Users/<username>/.pyenv/shims/black`
and this will happen:

```
pyenv: black: command not found

The `black' command exists in these Python versions:
3.7.2/envs/samcli37
samcli37
```

A simple workaround is to use `/Users/<username>/.pyenv/versions/samcli37/bin/black`
instead of `/Users/<username>/.pyenv/shims/black`.

#### Pre-commit
If you don't wish to manually run black on each pr or install black manually, we have integrated black into git hooks through [pre-commit](https://pre-commit.com/).
Expand Down
4 changes: 2 additions & 2 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -40,10 +40,10 @@ lint:
dev: lint test

black:
black samcli/* tests/*
black setup.py samcli tests

black-check:
black --check samcli/* tests/*
black --check setup.py samcli tests

# Verifications to run before sending a pull request
pr: init dev black-check
Expand Down
7 changes: 1 addition & 6 deletions appveyor.yml
Original file line number Diff line number Diff line change
Expand Up @@ -99,11 +99,6 @@ for:
- sh: "sudo unzip -d /opt/gradle /tmp/gradle-*.zip"
- sh: "PATH=/opt/gradle/gradle-5.5/bin:$PATH"

# Install black
- sh: "wget -O /tmp/black https://github.com/python/black/releases/download/19.10b0/black"
- sh: "chmod +x /tmp/black"
- sh: "/tmp/black --version"

# Install AWS CLI
- sh: "virtualenv aws_cli"
- sh: "./aws_cli/bin/python -m pip install awscli"
Expand Down Expand Up @@ -139,7 +134,7 @@ for:
# Runs only in Linux
- sh: "pytest -vv tests/integration"
- sh: "pytest -vv tests/regression"
- sh: "/tmp/black --check setup.py tests samcli"
- sh: "black --check setup.py tests samcli"

# Set JAVA_HOME to java11
- sh: "JAVA_HOME=/usr/lib/jvm/java-11-openjdk-amd64"
Expand Down
5 changes: 4 additions & 1 deletion requirements/dev.txt
Original file line number Diff line number Diff line change
Expand Up @@ -8,4 +8,7 @@ parameterized==0.7.0
pytest-xdist==1.30.0
pytest-forked==1.1.3
pytest-timeout==1.3.3
pytest-rerunfailures==7.0
pytest-rerunfailures==7.0

# formatter
black==20.8b1
Copy link
Contributor

Choose a reason for hiding this comment

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

I like that we have this effectively defined, instead of relying on user to have a version of black installed. This has caused conflicts in the past, with files being formatted with different versions of black.

8 changes: 7 additions & 1 deletion samcli/cli/cli_config_file.py
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,13 @@ def configuration_callback(cmd_name, option_name, saved_callback, provider, ctx,
config_dir = getattr(ctx, "samconfig_dir", None) or os.getcwd()
# If --config-file is an absolute path, use it, if not, start from config_dir
config_file_name = config_file if os.path.isabs(config_file) else os.path.join(config_dir, config_file)
config = get_ctx_defaults(cmd_name, provider, ctx, config_env_name=config_env_name, config_file=config_file_name,)
config = get_ctx_defaults(
cmd_name,
provider,
ctx,
config_env_name=config_env_name,
config_file=config_file_name,
)
ctx.default_map.update(config)

return saved_callback(ctx, param, config_env_name) if saved_callback else config_env_name
Expand Down
4 changes: 3 additions & 1 deletion samcli/commands/deploy/guided_context.py
Original file line number Diff line number Diff line change
Expand Up @@ -194,7 +194,9 @@ def run(self):
raise GuidedDeployFailedError(str(ex)) from ex

guided_config = GuidedConfig(template_file=self.template_file, section=self.config_section)
guided_config.read_config_showcase(self.config_file or DEFAULT_CONFIG_FILE_NAME,)
guided_config.read_config_showcase(
self.config_file or DEFAULT_CONFIG_FILE_NAME,
)

self.guided_prompts(_parameter_override_keys)

Expand Down
10 changes: 9 additions & 1 deletion samcli/commands/logs/command.py
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,15 @@
@pass_context
@track_command
def cli(
ctx, name, stack_name, filter, tail, start_time, end_time, config_file, config_env,
ctx,
name,
stack_name,
filter,
tail,
start_time,
end_time,
config_file,
config_env,
): # pylint: disable=redefined-builtin
# All logic must be implemented in the ``do_cli`` method. This helps with easy unit testing

Expand Down
6 changes: 5 additions & 1 deletion samcli/commands/publish/command.py
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,11 @@
@pass_context
@track_command
def cli(
ctx, template_file, semantic_version, config_file, config_env,
ctx,
template_file,
semantic_version,
config_file,
config_env,
):
# All logic must be implemented in the ``do_cli`` method. This helps with easy unit testing

Expand Down
5 changes: 4 additions & 1 deletion samcli/commands/validate/validate.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,10 @@
@pass_context
@track_command
def cli(
ctx, template_file, config_file, config_env,
ctx,
template_file,
config_file,
config_env,
):

# All logic must be implemented in the ``do_cli`` method. This helps with easy unit testing
Expand Down
14 changes: 12 additions & 2 deletions samcli/local/docker/lambda_debug_settings.py
Original file line number Diff line number Diff line change
Expand Up @@ -70,14 +70,24 @@ def get_debug_settings(debug_port, debug_args_list, runtime, options):
debug_env_vars={},
),
Runtime.nodejs10x.value: DebugSettings(
entry + ["/var/lang/bin/node"] + debug_args_list + ["/var/runtime/index.js",],
entry
+ ["/var/lang/bin/node"]
+ debug_args_list
+ [
"/var/runtime/index.js",
],
debug_env_vars={
"NODE_PATH": "/opt/nodejs/node_modules:/opt/nodejs/node10/node_modules:/var/runtime/node_module",
"NODE_OPTIONS": f"--inspect-brk=0.0.0.0:{str(debug_port)} --no-lazy --expose-gc --max-http-header-size 81920",
},
),
Runtime.nodejs12x.value: DebugSettings(
entry + ["/var/lang/bin/node"] + debug_args_list + ["/var/runtime/index.js",],
entry
+ ["/var/lang/bin/node"]
+ debug_args_list
+ [
"/var/runtime/index.js",
],
debug_env_vars={
"NODE_PATH": "/opt/nodejs/node_modules:/opt/nodejs/node12/node_modules:/var/runtime/node_module",
"NODE_OPTIONS": f"--inspect-brk=0.0.0.0:{str(debug_port)} --no-lazy --expose-gc --max-http-header-size 81920",
Expand Down
6 changes: 4 additions & 2 deletions tests/integration/buildcmd/build_integ_base.py
Original file line number Diff line number Diff line change
Expand Up @@ -163,7 +163,8 @@ def _test_with_default_gemfile(self, runtime, use_container, code_uri, relative_
"OtherRelativePathResource",
"BodyS3Location",
os.path.relpath(
os.path.normpath(os.path.join(str(relative_path), "SomeRelativePath")), str(self.default_build_dir),
os.path.normpath(os.path.join(str(relative_path), "SomeRelativePath")),
str(self.default_build_dir),
),
)

Expand All @@ -172,7 +173,8 @@ def _test_with_default_gemfile(self, runtime, use_container, code_uri, relative_
"GlueResource",
"Command.ScriptLocation",
os.path.relpath(
os.path.normpath(os.path.join(str(relative_path), "SomeRelativePath")), str(self.default_build_dir),
os.path.normpath(os.path.join(str(relative_path), "SomeRelativePath")),
str(self.default_build_dir),
),
)

Expand Down
6 changes: 4 additions & 2 deletions tests/integration/package/test_package_command.py
Original file line number Diff line number Diff line change
Expand Up @@ -466,11 +466,13 @@ def test_package_with_no_progressbar(self, no_progressbar):
upload_message = bytes("Uploading to", encoding="utf-8")
if no_progressbar:
self.assertNotIn(
upload_message, process_stderr,
upload_message,
process_stderr,
)
else:
self.assertIn(
upload_message, process_stderr,
upload_message,
process_stderr,
)

@parameterized.expand(
Expand Down
9 changes: 8 additions & 1 deletion tests/unit/commands/deploy/test_guided_context.py
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,14 @@ def test_guided_prompts_check_defaults_public_resources(
[
param((("CAPABILITY_IAM",),)),
param((("CAPABILITY_AUTO_EXPAND",),)),
param((("CAPABILITY_AUTO_EXPAND", "CAPABILITY_IAM",),)),
param(
(
(
"CAPABILITY_AUTO_EXPAND",
"CAPABILITY_IAM",
),
)
),
]
)
@patch("samcli.commands.deploy.guided_context.prompt")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -159,7 +159,10 @@ def test_subcommand_get_command_return_value(self, click_mock, functools_mock, o
s = EventTypeSubCommand(self.events_lib_mock, "hello", all_commands)
s.get_command(None, "hi")
click_mock.Command.assert_called_once_with(
name="hi", short_help="Generates a hello Event", params=[], callback=callback_object_mock,
name="hi",
short_help="Generates a hello Event",
params=[],
callback=callback_object_mock,
)

def test_subcommand_list_return_value(self):
Expand Down
6 changes: 4 additions & 2 deletions tests/unit/lib/samconfig/test_samconfig.py
Original file line number Diff line number Diff line change
Expand Up @@ -100,10 +100,12 @@ def test_add_params_from_different_sections(self):
self.samconfig.flush()
self._check_config_file()
self.assertEqual(
{"testKey1": True}, self.samconfig.get_all(cmd_names=["myCommand"], section="mySection1", env="myEnv"),
{"testKey1": True},
self.samconfig.get_all(cmd_names=["myCommand"], section="mySection1", env="myEnv"),
)
self.assertEqual(
{"testKey2": False}, self.samconfig.get_all(cmd_names=["myCommand"], section="mySection2", env="myEnv"),
{"testKey2": False},
self.samconfig.get_all(cmd_names=["myCommand"], section="mySection2", env="myEnv"),
)

def test_add_params_from_different_keys(self):
Expand Down
18 changes: 16 additions & 2 deletions tests/unit/lib/utils/test_hash.py
Original file line number Diff line number Diff line change
Expand Up @@ -38,13 +38,27 @@ def test_dir_hash_independent_of_file_order(self):
dir_checksums = {}
with patch("os.walk") as mockwalk:
mockwalk.return_value = [
(self.temp_dir, (), (file1.name, file2.name,),),
(
self.temp_dir,
(),
(
file1.name,
file2.name,
),
),
]
dir_checksums["first"] = dir_checksum(self.temp_dir)

with patch("os.walk") as mockwalk:
mockwalk.return_value = [
(self.temp_dir, (), (file2.name, file1.name,),),
(
self.temp_dir,
(),
(
file2.name,
file1.name,
),
),
]
dir_checksums["second"] = dir_checksum(self.temp_dir)

Expand Down