python-babel / babel

The official repository for Babel, the Python Internationalization Library
http://babel.pocoo.org/
BSD 3-Clause "New" or "Revised" License
1.29k stars 432 forks source link

Fix for #832 #1052

Open Edwin18 opened 6 months ago

Edwin18 commented 6 months ago

frontend.py: Fix function argument

Previosly in function was passed raw value self.ignore_dirs, not a list as expected ignore_dirs.

akx commented 6 months ago

Thanks! It would be nice to see a test for this, since the tests I wrote for #832 clearly didn't catch this typo 😅

codecov[bot] commented 6 months ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 90.99%. Comparing base (e0d1018) to head (f8f15ef).

:exclamation: Current head f8f15ef differs from pull request most recent head a6c692b. Consider uploading reports for the commit a6c692b to get more accurate results

Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #1052 +/- ## ======================================= Coverage 90.99% 90.99% ======================================= Files 26 26 Lines 4444 4444 ======================================= Hits 4044 4044 Misses 400 400 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

Edwin18 commented 6 months ago

Okay i will check what i can do :wink:

Edwin18 commented 6 months ago

Very simple minor fix 711676b

To make test work properly just add another value to our ignore list and it imidiately fails. Multiple args works fine: --ignore-dirs '_*' --ignore-dirs '*ignored*' but not multiple values: --ignore-dirs '*ignored* _*'

Before, function argument fix:

=============test session starts=============
platform linux -- Python 3.10.11, pytest-7.4.3, pluggy-1.3.0 -- /home/mrgreen/code/babel/venv/bin/python3
cachedir: .pytest_cache
rootdir: /home/mrgreen/code/babel
configfile: setup.cfg
plugins: cov-4.1.0
collected 60 items / 58 deselected / 2 selected                                                                                                                                                                                                                                                         

tests/messages/test_frontend.py::test_extract_ignore_dirs[False] 
ignore_dirs = ['*ignored*', '.*'] 
FAILED
tests/messages/test_frontend.py::test_extract_ignore_dirs[True] 
ignore_dirs = ['*ignored*', '.*', '_*'] 
FAILED

=============FAILURES =============
______________________ test_extract_ignore_dirs[False] ______________________

monkeypatch = <_pytest.monkeypatch.MonkeyPatch object at 0x7febbf363280>, capsys = <_pytest.capture.CaptureFixture object at 0x7febbf363520>, tmp_path = PosixPath('/tmp/pytest-of-mrgreen/pytest-33/test_extract_ignore_dirs_False0'), with_underscore_ignore = False

    @pytest.mark.parametrize("with_underscore_ignore", (False, True))
    def test_extract_ignore_dirs(monkeypatch, capsys, tmp_path, with_underscore_ignore):
        pot_file = tmp_path / 'temp.pot'
        monkeypatch.chdir(project_dir)
        cmd = f"extract . -o '{pot_file}' --ignore-dirs '*ignored* .*' "
        if with_underscore_ignore:
            # This also tests that multiple arguments are supported.
            cmd += "--ignore-dirs '_*'"
        cmdinst = configure_cli_command(cmd)
        assert isinstance(cmdinst, ExtractMessages)
        assert cmdinst.directory_filter
        cmdinst.run()
        pot_content = pot_file.read_text()

        # The `ignored` directory is now actually ignored:
>       assert 'this_wont_normally_be_here' not in pot_content
E       assert 'this_wont_normally_be_here' not in '# Translati...tr[1] ""\n\n'
E         'this_wont_normally_be_here' is contained here:
E           # Translations template for PROJECT.
E           # Copyright (C) 2023 ORGANIZATION
E           # This file is distributed under the same license as the PROJECT project.
E           # FIRST AUTHOR <EMAIL@ADDRESS>, 2023.
E           #
E           #, fuzzy...
E         
E         ...Full output truncated (33 lines hidden), use '-vv' to show

/home/mrgreen/code/babel/tests/messages/test_frontend.py:1588: AssertionError
______________________ test_extract_ignore_dirs[True] ______________________

monkeypatch = <_pytest.monkeypatch.MonkeyPatch object at 0x7febbeebbcd0>, capsys = <_pytest.capture.CaptureFixture object at 0x7febbeebb550>, tmp_path = PosixPath('/tmp/pytest-of-mrgreen/pytest-33/test_extract_ignore_dirs_True_0'), with_underscore_ignore = True

    @pytest.mark.parametrize("with_underscore_ignore", (False, True))
    def test_extract_ignore_dirs(monkeypatch, capsys, tmp_path, with_underscore_ignore):
        pot_file = tmp_path / 'temp.pot'
        monkeypatch.chdir(project_dir)
        cmd = f"extract . -o '{pot_file}' --ignore-dirs '*ignored* .*' "
        if with_underscore_ignore:
            # This also tests that multiple arguments are supported.
            cmd += "--ignore-dirs '_*'"
        cmdinst = configure_cli_command(cmd)
        assert isinstance(cmdinst, ExtractMessages)
        assert cmdinst.directory_filter
        cmdinst.run()
        pot_content = pot_file.read_text()

        # The `ignored` directory is now actually ignored:
>       assert 'this_wont_normally_be_here' not in pot_content
E       assert 'this_wont_normally_be_here' not in '# Translati...tr[1] ""\n\n'
E         'this_wont_normally_be_here' is contained here:
E           # Translations template for PROJECT.
E           # Copyright (C) 2023 ORGANIZATION
E           # This file is distributed under the same license as the PROJECT project.
E           # FIRST AUTHOR <EMAIL@ADDRESS>, 2023.
E           #
E           #, fuzzy...
E         
E         ...Full output truncated (29 lines hidden), use '-vv' to show

/home/mrgreen/code/babel/tests/messages/test_frontend.py:1588: AssertionError

After, function argument fix:

============= test session starts =============
platform linux -- Python 3.10.11, pytest-7.4.3, pluggy-1.3.0 -- /home/mrgreen/code/babel/venv/bin/python3
cachedir: .pytest_cache
rootdir: /home/mrgreen/code/babel
configfile: setup.cfg
plugins: cov-4.1.0
collected 60 items / 58 deselected / 2 selected                                                                                                                                                                                                                                                         

tests/messages/test_frontend.py::test_extract_ignore_dirs[False] 
ignore_dirs = ['*ignored*', '.*'] 
PASSED
tests/messages/test_frontend.py::test_extract_ignore_dirs[True] 
ignore_dirs = ['*ignored*', '.*', '_*'] 
PASSED
Edwin18 commented 6 months ago

And i have a question/suggestion about this one.

https://github.com/python-babel/babel/blob/2a1709a7768f6f07c3d2dbfdb03d3c8a6bd80aef/babel/messages/extract.py#L112-L115

https://github.com/python-babel/babel/blob/2a1709a7768f6f07c3d2dbfdb03d3c8a6bd80aef/babel/messages/extract.py#L201-L202

So in current realization we can have or default or user ignore list, maybe we should extend default one with user if it provided?

def _make_directory_filter(ignore_patterns):
    """
    Build a directory_filter function based on a list of ignore patterns.
    """

    if not ignore_patterns:
        ignore_patterns = []

    def cli_directory_filter(dirname):
        basename = os.path.basename(dirname)
        return not any(
            fnmatch.fnmatch(basename, ignore_pattern)
            for ignore_pattern
            in [*ignore_patterns, '.*', '_*']
        )

    return cli_directory_filter