Skip to content

Add filename construct validation #432

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.

Sign up for GitHub

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

Merged
merged 2 commits into from
Dec 15, 2024
Merged

Add filename construct validation #432

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
2167c54
add filename construct validation
jjw24 Dec 15, 2024
1c248a1
Merge branch 'plugin_api_v2' into add_filename_construct_validation
jjw24 Dec 15, 2024
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
5 changes: 4 additions & 1 deletion ci/src/_utils.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,10 @@ def plugin_reader() -> P:
return manifests

def get_plugin_files() -> list[str]:
return [os.path.join(plugin_dir, file) for file in os.listdir(plugin_dir)]
return [os.path.join(plugin_dir, filename) for filename in get_plugin_filenames()]

def get_plugin_filenames() -> list[str]:
return [file for file in os.listdir(plugin_dir)]
Copy link
Member

@Yusyuriv Yusyuriv Dec 15, 2024

Choose a reason for hiding this comment

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

It looks like you don't need a list comprehension in get_plugin_filenames as they return the same data, but list comprehension does slightly more work as it iterates through list one more time:

>>> [v for v in os.listdir('test')]
['script2.py', 'plugin.json', 'file.txt', 'script.py']
>>> os.listdir('test')
['script2.py', 'plugin.json', 'file.txt', 'script.py']
Suggested change
return [file for file in os.listdir(plugin_dir)]
return os.listdir(plugin_dir)

Copy link
Member

@Yusyuriv Yusyuriv Dec 15, 2024

Choose a reason for hiding this comment

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

I didn't notice the PR was on auto-merge. Oh well.

Copy link
Member Author

@jjw24 jjw24 Dec 16, 2024

Choose a reason for hiding this comment

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

Ah yes, my mistake, will correct it in the next PR.


def etag_reader() -> ETagsType:
with open(etag_file, "r", encoding="utf-8") as f:
Expand Down
9 changes: 8 additions & 1 deletion ci/src/validator.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
# -*-coding: utf-8 -*-
from _utils import clean, id_name, language_list, language_name, plugin_reader, check_url, icon_path, get_plugin_files
from _utils import clean, id_name, language_list, language_name, plugin_reader, check_url, icon_path, get_plugin_files, get_plugin_filenames

plugin_infos = plugin_reader()

Expand Down Expand Up @@ -28,3 +28,10 @@ def test_file_type_json():
incorrect_ext_files = [file for file in get_plugin_files() if not file.endswith(".json")]

assert len(incorrect_ext_files) == 0, f"Expected the following file to be of .json extension: {incorrect_ext_files}"

def test_file_name_construct():
filenames = get_plugin_filenames()
for info in plugin_infos:
assert (
f"{info['Name']}-{info['ID']}.json" in filenames
), f"Plugin {info['Name']} with ID {info['ID']} does not have the correct filename. Make sure it's name + ID, i.e. {info['Name']}-{info['ID']}.json"
Loading