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

tests/TaskEventTest: AssertionError: ev_start not called #488

Closed
arkamar opened this issue Sep 9, 2022 · 2 comments · Fixed by #502
Closed

tests/TaskEventTest: AssertionError: ev_start not called #488

arkamar opened this issue Sep 9, 2022 · 2 comments · Fixed by #502
Assignees
Milestone

Comments

@arkamar
Copy link
Contributor

arkamar commented Sep 9, 2022

I am experiencing weird behavior of tests when I try to run just subset of tests. For example

nosetests -v --all-modules tests/TaskEventTest.py

all tests pass but

nosetests -v --all-modules tests/StreamWorkerTest.py tests/TaskEventTest.py

do not. It fails with ev_start related errors (see https://github.com/arkamar/clustershell/runs/8268340929?check_suite_focus=true#step:9:40):

======================================================================
FAIL: test ev_pickup event with fanout
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/runner/work/clustershell/clustershell/tests/TaskEventTest.py", line 516, in test_ev_pickup_fanout
    eh.do_asserts_noread_notimeout()
  File "/home/runner/work/clustershell/clustershell/tests/TaskEventTest.py", line 40, in do_asserts_noread_notimeout
    assert self.did_start, "ev_start not called"
AssertionError: ev_start not called

======================================================================
FAIL: test ev_pickup event with fanout (legacy)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/runner/work/clustershell/clustershell/tests/TaskEventTest.py", line 495, in test_ev_pickup_fanout_legacy
    eh.do_asserts_noread_notimeout()
  File "/home/runner/work/clustershell/clustershell/tests/TaskEventTest.py", line 40, in do_asserts_noread_notimeout
    assert self.did_start, "ev_start not called"
AssertionError: ev_start not called

Interestingly, if I switch order of files

nosetests -v --all-modules tests/TaskEventTest.py tests/StreamWorkerTest.py

everything works again.

arkamar added a commit to arkamar/gentoo that referenced this issue Sep 11, 2022
Multiple aspects of the ebuild were changed from previous versions:
- it is converted to new PEP517 mode
- it supports py3.9, py3.10 and py3.11
- homepage link is fixed
- it uses .gh.tar.gz suffix for source archive as suggested in Gentoo
  Python Guide
- the doc use is removed because it is not used anymore since commit
  3ca0636 ("app-admin/clustershell: version bump.")
- dev-libs/openssl dependency is removed, it is not used in sources code
- working tests - most of tests work with an exception of those
  requiring remote ssh connections, which are removed in prepare step.
  It was necessary to apply few fixes [1-3] taken from upstream in order
  to make them work. The tests are executed with unittest rather than
  nose which makes py3.11 support possible.
- it installs man pages again, this is basically resurrected from commit
  3ca0636 ("app-admin/clustershell: version bump.")
- unlike legacy mode, PEP517 installs etc/* files to /usr/etc, hence it
  was necessary to correct installation path with move (I didn't find
  any better solution, this one is used across repository)

[1] cea-hpc/clustershell@314767d
[2] cea-hpc/clustershell#487
[3] cea-hpc/clustershell#488

Closes: https://bugs.gentoo.org/866071
Bug: https://bugs.gentoo.org/845426
Signed-off-by: Petr Vaněk <arkamar@atlas.cz>
arkamar added a commit to arkamar/gentoo that referenced this issue Oct 4, 2022
Multiple aspects of the ebuild were changed from previous versions:
- it is converted to new PEP517 mode
- it supports py3.9, py3.10 and py3.11
- homepage link is fixed
- it uses .gh.tar.gz suffix for source archive as suggested in Gentoo
  Python Guide
- the doc use is removed because it is not used anymore since commit
  3ca0636 ("app-admin/clustershell: version bump.")
- dev-libs/openssl dependency is removed, it is not used in sources code
- working tests - most of tests work with an exception of those
  requiring remote ssh connections, which are removed in prepare step.
  It was necessary to apply few fixes [1-3] taken from upstream in order
  to make them work. The tests are executed with unittest rather than
  nose which makes py3.11 support possible.
- it installs man pages again, this is basically resurrected from commit
  3ca0636 ("app-admin/clustershell: version bump.")
- unlike legacy mode, PEP517 installs etc/* files to /usr/etc, hence it
  was necessary to correct installation path with move (I didn't find
  any better solution, this one is used across repository)

[1] cea-hpc/clustershell@314767d
[2] cea-hpc/clustershell#487
[3] cea-hpc/clustershell#488

Closes: https://bugs.gentoo.org/866071
Bug: https://bugs.gentoo.org/845426
Signed-off-by: Petr Vaněk <arkamar@atlas.cz>
arkamar added a commit to arkamar/gentoo that referenced this issue Oct 4, 2022
Multiple aspects of the ebuild were changed from previous versions:
- it is converted to new PEP517 mode
- it supports py3.9, py3.10 and py3.11
- homepage link is fixed
- it uses .gh.tar.gz suffix for source archive as suggested in Gentoo
  Python Guide
- the doc use is removed because it is not used anymore since commit
  3ca0636 ("app-admin/clustershell: version bump.")
- dev-libs/openssl dependency is removed, it is not used in sources code
- working tests - most of tests work with an exception of those
  requiring remote ssh connections, which are removed in prepare step.
  It was necessary to apply few fixes [1-3] taken from upstream in order
  to make them work. The tests are executed with unittest rather than
  nose which makes py3.11 support possible.
- it installs man pages again, this is basically resurrected from commit
  3ca0636 ("app-admin/clustershell: version bump.")
- unlike legacy mode, PEP517 installs etc/* files to /usr/etc, hence it
  was necessary to correct installation path with move (I didn't find
  any better solution, this one is used across repository)

[1] cea-hpc/clustershell@314767d
[2] cea-hpc/clustershell#487
[3] cea-hpc/clustershell#488

Closes: https://bugs.gentoo.org/866071
Bug: https://bugs.gentoo.org/845426
Signed-off-by: Petr Vaněk <arkamar@atlas.cz>
gentoo-bot pushed a commit to gentoo/gentoo that referenced this issue Oct 4, 2022
Multiple aspects of the ebuild were changed from previous versions:
- it is converted to new PEP517 mode
- it supports py3.9, py3.10 and py3.11
- homepage link is fixed
- it uses .gh.tar.gz suffix for source archive as suggested in Gentoo
  Python Guide
- the doc use is removed because it is not used anymore since commit
  3ca0636 ("app-admin/clustershell: version bump.")
- dev-libs/openssl dependency is removed, it is not used in sources code
- working tests - most of tests work with an exception of those
  requiring remote ssh connections, which are removed in prepare step.
  It was necessary to apply few fixes [1-3] taken from upstream in order
  to make them work. The tests are executed with unittest rather than
  nose which makes py3.11 support possible.
- it installs man pages again, this is basically resurrected from commit
  3ca0636 ("app-admin/clustershell: version bump.")
- unlike legacy mode, PEP517 installs etc/* files to /usr/etc, hence it
  was necessary to correct installation path with move (I didn't find
  any better solution, this one is used across repository)

[1] cea-hpc/clustershell@314767d
[2] cea-hpc/clustershell#487
[3] cea-hpc/clustershell#488

Closes: https://bugs.gentoo.org/866071
Bug: https://bugs.gentoo.org/845426
Signed-off-by: Petr Vaněk <arkamar@atlas.cz>
Signed-off-by: Sam James <sam@gentoo.org>
@thiell thiell self-assigned this Nov 25, 2022
@thiell thiell added this to the 1.9 milestone Nov 25, 2022
thiell added a commit to thiell/clustershell that referenced this issue Nov 26, 2022
An unconfigured StreamWorker does not have streams to register in the
engine, so when registered, it stays until it is explicitly aborted. To
make that worse, it is using a 'fanout slot' and if the Engine's fanout
is too low, it can lead to the engine run loop exiting prematurely. This
is not a case handled very well by the Engine. See cea-hpc#497 for more details
about the problem.

In the meantime, we fix the test to exit with a clean engine state.

Fixes cea-hpc#488.
@thiell
Copy link
Collaborator

thiell commented Nov 26, 2022

Thank you, @arkamar!

thiell added a commit that referenced this issue Nov 26, 2022
An unconfigured StreamWorker does not have streams to register in the
engine, so when registered, it stays until it is explicitly aborted. To
make that worse, it is using a 'fanout slot' and if the Engine's fanout
is too low, it can lead to the engine run loop exiting prematurely. This
is not a case handled very well by the Engine. See #497 for more details
about the problem.

In the meantime, we fix the test to exit with a clean engine state.

Fixes #488.
arkamar added a commit to arkamar/gentoo that referenced this issue Nov 27, 2022
Patches are not needed anymore. Upstream have fixed reported issue [1]
and also worked in conditional unittest skips for hostname set to
localhost [2], thus TreeWrokerTest.py does not need to be removed in
prepare phase.

[1] cea-hpc/clustershell#488
[2] cea-hpc/clustershell@aae7bc7

Signed-off-by: Petr Vaněk <arkamar@atlas.cz>
@arkamar
Copy link
Contributor Author

arkamar commented Nov 27, 2022

@thiell Thanks for the fix and also for aae7bc7, it is handy for Gentoo package!

gentoo-bot pushed a commit to gentoo/gentoo that referenced this issue Dec 6, 2022
Patches are not needed anymore. Upstream have fixed reported issue [1]
and also worked in conditional unittest skips for hostname set to
localhost [2], thus TreeWrokerTest.py does not need to be removed in
prepare phase.

[1] cea-hpc/clustershell#488
[2] cea-hpc/clustershell@aae7bc7

Signed-off-by: Petr Vaněk <arkamar@atlas.cz>
Signed-off-by: Sam James <sam@gentoo.org>
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