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

unit tests fail on windows runners #60

Open
Javagedes opened this issue Oct 24, 2023 · 2 comments · May be fixed by #61
Open

unit tests fail on windows runners #60

Javagedes opened this issue Oct 24, 2023 · 2 comments · May be fixed by #61

Comments

@Javagedes
Copy link

Hello, unit tests fail on windows runners, specifically for the trailing whitespace tests. This occured for the recent changes made to fix issues with resolving symlinks. The Path call in _normalize_path retains whitespaces on Linux, but removes them on Windows.

A simple (but not elegant) fix I found was to re-apply the trailing whitespace on windows systems.

# At bottom of file
def _count_trailing_whitespace(text: str):
    count = 0
    for char in reversed(str(text)):
        if char.isspace():
            count += 1
        else:
            break
    return count
# In IgnoreRule
def match(self, abs_path):
    """Returns True or False if the path matches the rule."""
    matched = False
    if self.base_path:
        rel_path = str(_normalize_path(abs_path).relative_to(self.base_path))
    else:
        rel_path = str(_normalize_path(abs_path))
    
    # Path() strips trailing spaces on windows
    if sys.platform.startswith('win'):        
        rel_path += " " * _count_trailing_whitespace(abs_path)
    
    # Path() strips the trailing slash, so we need to preserve it
    # in case of directory-only negation
    if self.negation and isinstance(abs_path, str) and abs_path[-1] == '/':
        rel_path += '/'
    if rel_path.startswith('./'):
        rel_path = rel_path[2:]
    if re.search(self.regex, rel_path):
        matched = True
    return matched
@mherrmann
Copy link
Owner

Hello, as usual I'll be happy to accept a PR that fixes this problem.

@Javagedes Javagedes linked a pull request Oct 25, 2023 that will close this issue
@Javagedes
Copy link
Author

Hello, as usual I'll be happy to accept a PR that fixes this problem.

Added #61

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants