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

[V - Non-Rule] Rule-specific testing #51

Merged
merged 6 commits into from
Sep 18, 2023
Merged
Changes from 2 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
6 changes: 5 additions & 1 deletion test/test_main.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import os
import glob
import sys
import re

import pytest
import tabulate
Expand All @@ -11,7 +12,10 @@
sys.path.insert(0, os.path.join(os.path.dirname(__file__), '..'))
from main import run

test_files = glob.glob(os.path.join(os.path.dirname(__file__), "files/**/*.ifc"), recursive=True)
test_for_rules = any(re.match(r"^[a-zA-Z]{3}\d{3}$", arg) for arg in sys.argv[1:])
test_files = [fn for fn in glob.glob(os.path.join(os.path.dirname(__file__),
Copy link
Collaborator

Choose a reason for hiding this comment

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

I still find it a bit confusing:

  • test_for_rules: why is this a boolean, why not do filter() instead of any() to filter out all non-rule code argv and use them below
  • the regex is lower or upper case but by doing x in fn.lower() actually an uppercase argument will not work
  • some of the rule codes such as alb will arbitrarily match word fragments such as albino which will be rather confusing. Test files are in separate directories so why not filter only on that directory name instead of entire file path?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's modified in 377f4e6

why is this a boolean, why not do filter() instead of any() to filter out all non-rule code argv and use them below

Done.

some of the rule codes such as alb will arbitrarily match word fragments such as albino which will be rather confusing. Test files are in separate directories so why not filter only on that directory name instead of entire file path?

Done.

the regex is lower or upper case but by doing x in fn.lower() actually an uppercase argument will not work

The aim was to also allow for uppercase input, such as 'Alb001' or'ALB001'instead of 'alb001' It should be good now: the regex match will still allow for this uppercase and it later converts into lowercases to match the folder name (which are always in lowercases, such as in 'alb001').

Furthermore, there is a warning added in the case there is someone wants to test a non-existing rule code (e.g. 'alb005'). This could be either a typo or it could mean that there are no test files added yet (or something else even). Nothing too spectecular, but it might help someone writing rules and testing it locally.

"files/**/*.ifc"), recursive=True) if len(sys.argv) < 2 or any(x in fn.lower() for x in sys.argv[1:]) or not test_for_rules]

@pytest.mark.parametrize("filename", test_files)
def test_invocation(filename):
results = list(run(filename))
Expand Down