Fixed#2591: Replaced os.sep with '/' as it behaves differently on linux and windows.

[srinivasreddy]

Removed os.sep as it behaves differently linux and windows

• on linux it is '/'
• on windows it is '\\'

[coveralls]

Coverage remained the same at 92.059% when pulling 7c6d1c9 on srinivasreddy:2591 into 1a9bc14 on pytest-dev:master.

[coveralls]

Coverage remained the same at 92.059% when pulling 01f0e25 on srinivasreddy:2591 into 1a9bc14 on pytest-dev:master.

[The-Compiler]

This should have a test, and a changelog entry.

[nicoddemus]

I agree with @The-Compiler. 👍

To test it, given that this is a somewhat coupled part of the code with the rest of the machinery, I suggest to refactor the loop which identifies the packages/modules of the entrypoint:

         for fn in package_files:
              is_simple_module = '/' not in fn and fn.endswith('.py')
              is_package = fn.count('/') == 1 and fn.endswith('__init__.py')
              if is_simple_module:
                  module_name, ext = os.path.splitext(fn)
                  hook.mark_rewrite(module_name)
              elif is_package:
                  package_name = os.path.dirname(fn)
                  hook.mark_rewrite(package_name)

Into its own function so the above block can become:

for name in iter_rewritable_modules(package_files):
    hook.mark_rewrite(name)

This way you can easily unit-test the logic of iter_rewritable_modules.

[coveralls]

Coverage increased (+0.004%) to 92.064% when pulling eb78410 on srinivasreddy:2591 into 1a9bc14 on pytest-dev:master.

[nicoddemus]

Please add a test for the new iter_rewritable_modules function

[nicoddemus]

Now this just need a test 😉

[srinivasreddy]

@nicoddemus pointers please. I am new to pytest

[nicoddemus]

pointers please. I am new to pytest

Sure thing. 👍

I meant that now you can write a proper unit-test along this lines:

@pytest.mark.parametrize('names, expected', [
    (['foo', 'bar.py'], ['foo.bar']),
    (['foo', 'bar.pyc'], []),
    (['foo', '__init__.py'], ['foo']),
    (['foo', 'bar', '__init__.py'], []),
])
def test_iter_rewritable_modules(names, expected):
    from os.path import join
    assert list(iter_rewritable_modules(join(*names) == expected

Note that iter_rewritable_modules can be a function in _pytest/config.py instead of a method because it doesn't need anything from self. (Now I realize it should also be named _iter_rewritable_modules because it is not public)

[nicoddemus]

This also needs a changelog entry:

Write a file 2591.bugfix in the changelog/ folder with something like:

Correctly consider ``/`` as the file separator to automatically mark plugin files for rewrite on Windows.

[nicoddemus]

Please make this an actual generator instead of returning a list, replacing modules.append(module_name) by yield module_name.

[coveralls]

Coverage increased (+0.02%) to 91.824% when pulling 84c7414 on srinivasreddy:2591 into d5f4496 on pytest-dev:master.

[coveralls]

Coverage increased (+0.02%) to 91.824% when pulling a0101f0 on srinivasreddy:2591 into d5f4496 on pytest-dev:master.

[coveralls]

Coverage increased (+0.02%) to 91.824% when pulling a0101f0 on srinivasreddy:2591 into d5f4496 on pytest-dev:master.

[srinivasreddy]

@nicoddemus Please take a look. the build has been passed.

[nicoddemus]

Thanks @srinivasreddy!