-
Notifications
You must be signed in to change notification settings - Fork 25
Change config filenames #46
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
Changes from all commits
9f18d9c
8a98eb1
f045730
f493bee
6408dfb
02f86b0
b3fb431
968d2e5
3506e3e
56a0f7d
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -4,6 +4,7 @@ | |
| from typing import Optional | ||
|
|
||
| import jinja2 | ||
| import toml | ||
|
|
||
| from pyscript import config | ||
|
|
||
|
|
@@ -50,6 +51,10 @@ def create_project( | |
| app_dir.mkdir() | ||
| manifest_file = app_dir / config["project_config_filename"] | ||
| with manifest_file.open("w", encoding="utf-8") as fp: | ||
| json.dump(context, fp) | ||
| if str(manifest_file).endswith(".json"): | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Just wondering, what do you think about using fp.name instead of converting manifest_file to a string? I guess manifest_file is more readable tho (unless we change the
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good point... I think I'd lean more toward what you said in terms of readability...
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There is a method on Path,
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Naming nit? The constant is called "project_config_filename" but then we use "manifest_file" for the path? Maybe "project_config_path = ..."? |
||
| json.dump(context, fp) | ||
| else: | ||
| toml.dump(context, fp) | ||
|
|
||
| index_file = app_dir / "index.html" | ||
| string_to_html('print("Hello, world!")', app_name, index_file) | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -8,41 +8,126 @@ | |
| from typing import Any | ||
|
|
||
| import pytest | ||
| import toml | ||
|
|
||
| from pyscript import _generator as gen | ||
| from pyscript import config | ||
|
|
||
| TESTS_AUTHOR_NAME = "A.Coder" | ||
| TESTS_AUTHOR_EMAIL = "acoder@domain.com" | ||
|
|
||
|
|
||
| def test_create_project(tmp_cwd: Path, is_not_none: Any) -> None: | ||
| app_name = "app_name" | ||
| app_description = "A longer, human friendly, app description." | ||
| author_name = "A.Coder" | ||
| author_email = "acoder@domain.com" | ||
| gen.create_project(app_name, app_description, author_name, author_email) | ||
|
|
||
| manifest_path = tmp_cwd / app_name / config["project_config_filename"] | ||
| assert manifest_path.exists() | ||
| # GIVEN a a new project | ||
| gen.create_project(app_name, app_description, TESTS_AUTHOR_NAME, TESTS_AUTHOR_EMAIL) | ||
|
|
||
| with manifest_path.open() as fp: | ||
| contents = json.load(fp) | ||
| # with a default config path | ||
| manifest_path = tmp_cwd / app_name / config["project_config_filename"] | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ditto for previous comment about picking one name. |
||
|
|
||
| assert contents == { | ||
| "name": "app_name", | ||
| "description": "A longer, human friendly, app description.", | ||
| "type": "app", | ||
| "author_name": "A.Coder", | ||
| "author_email": "acoder@domain.com", | ||
| "version": is_not_none, | ||
| } | ||
| check_project_manifest(manifest_path, toml, app_name, is_not_none) | ||
|
|
||
|
|
||
| def test_create_project_twice_raises_error(tmp_cwd: Path) -> None: | ||
| """We get a FileExistsError when we try to create an existing project.""" | ||
| app_name = "app_name" | ||
| app_description = "A longer, human friendly, app description." | ||
| author_name = "A.Coder" | ||
| author_email = "acoder@domain.com" | ||
| gen.create_project(app_name, app_description, author_name, author_email) | ||
| gen.create_project(app_name, app_description, TESTS_AUTHOR_NAME, TESTS_AUTHOR_EMAIL) | ||
|
|
||
| with pytest.raises(FileExistsError): | ||
| gen.create_project(app_name, app_description, author_name, author_email) | ||
| gen.create_project( | ||
| app_name, app_description, TESTS_AUTHOR_NAME, TESTS_AUTHOR_EMAIL | ||
| ) | ||
|
|
||
|
|
||
| def test_create_project_explicit_json( | ||
| tmp_cwd: Path, is_not_none: Any, monkeypatch | ||
| ) -> None: | ||
| app_name = "JSON_app_name" | ||
| app_description = "A longer, human friendly, app description." | ||
|
|
||
| # Let's patch the config so that the project config file is a JSON file | ||
| config_file_name = "pyscript.json" | ||
| monkeypatch.setitem(gen.config, "project_config_filename", config_file_name) | ||
|
|
||
| # GIVEN a new project | ||
| gen.create_project(app_name, app_description, TESTS_AUTHOR_NAME, TESTS_AUTHOR_EMAIL) | ||
|
|
||
| # get the path where the config file is being created | ||
| manifest_path = tmp_cwd / app_name / config["project_config_filename"] | ||
|
|
||
| check_project_manifest(manifest_path, json, app_name, is_not_none) | ||
|
|
||
|
|
||
| def test_create_project_explicit_toml( | ||
| tmp_cwd: Path, is_not_none: Any, monkeypatch | ||
| ) -> None: | ||
| app_name = "TOML_app_name" | ||
| app_description = "A longer, human friendly, app description." | ||
|
|
||
| # Let's patch the config so that the project config file is a JSON file | ||
| config_file_name = "mypyscript.toml" | ||
| monkeypatch.setitem(gen.config, "project_config_filename", config_file_name) | ||
|
|
||
| # GIVEN a new project | ||
| gen.create_project(app_name, app_description, TESTS_AUTHOR_NAME, TESTS_AUTHOR_EMAIL) | ||
|
|
||
| # get the path where the config file is being created | ||
| manifest_path = tmp_cwd / app_name / config["project_config_filename"] | ||
|
|
||
| check_project_manifest(manifest_path, toml, app_name, is_not_none) | ||
|
|
||
|
|
||
| def check_project_manifest( | ||
| config_path: Path, | ||
| serializer: Any, | ||
| app_name: str, | ||
| is_not_none: Any, | ||
| app_description: str = "A longer, human friendly, app description.", | ||
| author_name: str = TESTS_AUTHOR_NAME, | ||
| author_email: str = TESTS_AUTHOR_EMAIL, | ||
| project_type: str = "app", | ||
| ): | ||
| """ | ||
| Perform the following: | ||
| * checks that `config_path` exists | ||
| * loads the contents of `config_path` using `serializer.load` | ||
| * check that the contents match with the values provided in input. Specifically: | ||
| * "name" == app_name | ||
| * "description" == app_description | ||
| * "type" == app_type | ||
| * "author_name" == author_name | ||
| * "author_email" == author_email | ||
| * "version" == is_not_none | ||
| Params: | ||
| * config_path(Path): path to the app config file | ||
| * serializer(json|toml): serializer to be used to load contents of `config_path`. | ||
| Supported values are either modules `json` or `toml` | ||
| * app_name(str): name of application | ||
| * is_not_none(any): pytest fixture | ||
| * app_description(str): application description | ||
| * author_name(str): application author name | ||
| * author_email(str): application author email | ||
| * project_type(str): project type | ||
| """ | ||
| # assert that the new project config file exists | ||
| assert config_path.exists() | ||
|
|
||
| # assert that we can load it as a TOML file (TOML is the default config format) | ||
| # and that the contents of the config are as we expect | ||
| with config_path.open() as fp: | ||
| contents = serializer.load(fp) | ||
|
|
||
| assert contents == { | ||
| "name": app_name, | ||
| "description": app_description, | ||
| "type": project_type, | ||
| "author_name": author_name, | ||
| "author_email": author_email, | ||
| "version": is_not_none, | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe a comment would help here to clear up the difference between the config file for the CLI itself, and the project config file? When I first looked at the code it took me a minute to see why DEFAULT_CONFIG_FILENAME didn't match the value in DEFAULT_CONFIG. Or maybe (although it is longer), always refer to them as either CLI_CONFIG_FILENAME, cli_config and PROJECT_... etc etc?