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

python3Packages.parse: 1.16.0 -> 1.18.0, python-docx: run behave tests #97737

Merged
merged 3 commits into from Sep 20, 2020

Conversation

maxxk
Copy link
Contributor

@maxxk maxxk commented Sep 11, 2020

Motivation for this change

Fix behave which was broken from previous parse update (see nixpkgs-review from #95690; behave/behave#862).

ZHF: #97479

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Ensured that relevant documentation is up to date
  • Fits CONTRIBUTING.md.

nixpkgs-review:

3 packages failed to build:
python37Packages.pytest-bdd python38Packages.pytest-bdd terraform-compliance

17 packages built:
nrfutil python27Packages.parse python27Packages.parse-type python27Packages.pyparser python27Packages.pytest-bdd python37Packages.behave python37Packages.parse python37Packages.parse-type python37Packages.pyparser python37Packages.python-docx python37Packages.radish-bdd python38Packages.behave python38Packages.parse python38Packages.parse-type python38Packages.pyparser python38Packages.python-docx python38Packages.radish-bdd

Failed packages also fail on master.

Previously the problematic "behave" dependency
was not even used at checkPhase.
@risicle
Copy link
Contributor

risicle commented Sep 12, 2020

behave fails to build for me on macos 10.14:

=================================== FAILURES ===================================
___ TestAsyncStepDecoratorPy34.test_step_decorator_async_run_until_complete2 ___

self = <tests.api._test_async_step34.TestAsyncStepDecoratorPy34 object at 0x1053c5150>

    def test_step_decorator_async_run_until_complete2(self):
        step_container = SimpleStepContainer()
        with use_step_import_modules(step_container):
            # -- STEP-DEFINITIONS EXAMPLE (as MODULE SNIPPET):
            # VARIANT 2: Use @asyncio.coroutine def step_impl()
            from behave import step
            from behave.api.async_step import async_run_until_complete
            import asyncio
    
            @step('a tagged-coroutine async step waits "{duration:f}" seconds')
            @async_run_until_complete
            @asyncio.coroutine
            def step_async_step_waits_seconds2(context, duration):
                yield from asyncio.sleep(duration)
    
        # -- USES: async def step_impl(...) as async-step (coroutine)
        AsyncStepTheory.validate(step_async_step_waits_seconds2)
    
        # -- RUN ASYNC-STEP: Verify that it is behaving correctly.
        # ENSURE: Execution of async-step matches expected duration.
        context = Context(runner=Runner(config={}))
        with StopWatch() as stop_watch:
            step_async_step_waits_seconds2(context, duration=0.2)
>       assert abs(stop_watch.duration - 0.2) <= 0.05
E       AssertionError

tests/api/_test_async_step34.py:75: AssertionError
___ TestAsyncStepDecoratorPy35.test_step_decorator_async_run_until_complete1 ___

self = <tests.api._test_async_step35.TestAsyncStepDecoratorPy35 object at 0x1051bd890>

    def test_step_decorator_async_run_until_complete1(self):
        step_container = SimpleStepContainer()
        with use_step_import_modules(step_container):
            # -- STEP-DEFINITIONS EXAMPLE (as MODULE SNIPPET):
            # VARIANT 1: Use async def step_impl()
            from behave import step
            from behave.api.async_step import async_run_until_complete
            import asyncio
    
            @step('an async coroutine step waits "{duration:f}" seconds')
            @async_run_until_complete
            async def step_async_step_waits_seconds(context, duration):
                await asyncio.sleep(duration)
    
        # -- USES: async def step_impl(...) as async-step (coroutine)
        AsyncStepTheory.validate(step_async_step_waits_seconds)
    
        # -- RUN ASYNC-STEP: Verify that it is behaving correctly.
        # ENSURE: Execution of async-step matches expected duration.
        context = Context(runner=Runner(config={}))
        with StopWatch() as stop_watch:
            step_async_step_waits_seconds(context, 0.2)
>       assert abs(stop_watch.duration - 0.2) <= 0.05
E       AssertionError

tests/api/_test_async_step35.py:75: AssertionError
=============================== warnings summary ===============================
tests/unit/test_behave4cmd_command_shell_proc.py:26
  /private/tmp/nix-build-behave-1.2.7.dev1.drv-1/source/tests/unit/test_behave4cmd_command_shell_proc.py:26: DeprecationWarning: invalid escape sequence \w
    winpath_pattern = u"^([A-Za-z]:(\\[\w\.\-]+)+)|((\\[\w\.\-]+)*)|(\s[\w\.\-]+([\w\.\-]+)*)$"

-- Docs: https://docs.pytest.org/en/latest/warnings.html
- generated xml file: /private/tmp/nix-build-behave-1.2.7.dev1.drv-1/source/build/testing/report.xml -
- generated html file: file:///private/tmp/nix-build-behave-1.2.7.dev1.drv-1/source/build/testing/report.html -
=========================== short test summary info ============================
FAILED tests/api/test_async_step.py::TestAsyncStepDecoratorPy34::test_step_decorator_async_run_until_complete2
FAILED tests/api/test_async_step.py::TestAsyncStepDecoratorPy35::test_step_decorator_async_run_until_complete1
======= 2 failed, 1248 passed, 7 skipped, 2 xfailed, 1 warning in 12.80s =======

This looks like its a timing-based test, which are always trouble - perhaps we should try and skip these tests?

Copy link
Contributor

@drewrisinger drewrisinger left a comment

Choose a reason for hiding this comment

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

  • Diff LGTM
  • Commits LGTM
  • Confirm nix-review results from author:
https://github.com/NixOS/nixpkgs/pull/97737
2 packages failed to build:
python38Packages.pytest-bdd terraform-compliance

18 packages built:
nrfutil python27Packages.parse python27Packages.parse-type python27Packages.pyparser python27Packages.pytest-bdd python37Packages.behave python37Packages.parse python37Packages.parse-type python37Packages.pyparser python37Packages.pytest-bdd python37Packages.python-docx python37Packages.radish-bdd python38Packages.behave python38Packages.parse python38Packages.parse-type python38Packages.pyparser python38Packages.python-docx python38Packages.radish-bdd

@maxxk
Copy link
Contributor Author

maxxk commented Sep 13, 2020

behave fails to build for me on macos 10.14:

@risicle I tried several times building both python38Packages.behave and, as in your example, python37Packages.behave but unfortunately I couldn't reproduce the issue using fresh Nix install on macOS 10.14.6 18G6020 :(

Probably we could ping someone maintaining nixpkgs on darwin to continue investigating?

In the meantime, could you please check that changing pytest command to the following fixes the build for you:
https://github.com/NixOS/nixpkgs/blob/master/pkgs/development/python-modules/behave/default.nix#L33

pytest tests -k "not test_step_decorator_async_run_until_complete"

@risicle
Copy link
Contributor

risicle commented Sep 13, 2020

I simply think I've got slower computers than you (maybe try building on a heavily loaded machine), and as I say, timing-based tests are just trouble sooner or later. It's not too much of a loss disabling just test_step_decorator_async_run_until_complete* is it?

@drewrisinger
Copy link
Contributor

You can disable only that test if using pytestCheckHook and disableTests

checkInputs = [ pytestCheckHook ... ];

disabledTests = lib.optionals stdenv.isDarwin [ "test_step_decorator_async_run_until_complete" ];

@maxxk
Copy link
Contributor Author

maxxk commented Sep 15, 2020

Thanks, I added the change. I'd just prefer to fix the test, at least by increasing the timeout, but skipping it on darwin is fine too.

Copy link
Contributor

@drewrisinger drewrisinger left a comment

Choose a reason for hiding this comment

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

  • Minor point in diff still: redundant pytest
  • Commits LGTM
  • Builds via nix-review: Failed packages fail on master
https://github.com/NixOS/nixpkgs/pull/97737
2 packages failed to build:
python38Packages.pytest-bdd terraform-compliance

18 packages built:
nrfutil python27Packages.parse python27Packages.parse-type python27Packages.pyparser python27Packages.pytest-bdd python37Packages.behave python37Packages.parse python37Packages.parse-type python37Packages.pyparser python37Packages.pytest-bdd python37Packages.python-docx python37Packages.radish-bdd python38Packages.behave python38Packages.parse python38Packages.parse-type python38Packages.pyparser python38Packages.python-docx python38Packages.radish-bdd

pkgs/development/python-modules/behave/default.nix Outdated Show resolved Hide resolved
pkgs/development/python-modules/behave/default.nix Outdated Show resolved Hide resolved
@jonringer
Copy link
Contributor

sorry @maxxk that this turned out to be such a ..... pain of a PR

but thanks for sticking through it :)

Copy link
Contributor

@jonringer jonringer left a comment

Choose a reason for hiding this comment

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

LGTM

failures are broken on target branch

https://github.com/NixOS/nixpkgs/pull/97737
3 packages failed to build:
python37Packages.pytest-bdd python38Packages.pytest-bdd terraform-compliance

17 packages built:
nrfutil python27Packages.parse python27Packages.parse-type python27Packages.pyparser python27Packages.pytest-bdd python37Packages.behave python37Packages.parse python37Packages.parse-type python37Packages.pyparser python37Packages.python-docx python37Packages.radish-bdd python38Packages.behave python38Packages.parse python38Packages.parse-type python38Packages.pyparser python38Packages.python-docx python38Packages.radish-bdd

@jonringer jonringer merged commit 0f34c4e into NixOS:master Sep 20, 2020
maxxk added a commit to maxxk/nixpkgs that referenced this pull request Sep 23, 2020
Timing-based test is flaky on Darwin:
NixOS#97737 (comment)

(cherry picked from commit 0f34c4e)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants