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

Fixes #771 allure-behave formatter crash with behave v1.2.7.dev5 #798

Merged
merged 6 commits into from
Mar 14, 2024
Merged

Conversation

ercaronte
Copy link
Contributor

@ercaronte ercaronte commented Mar 5, 2024

This is a fix for issue #771: with behave v1.2.7.dev5 an error causes the allure_behave report formatter to crash.

Context

With behave v1.2.7.dev5 the allure-behave python library crashes every run, with exceptions like the following example:

File "/Users/marcocarosi/Code/miniconda3/envs/phoenix3/lib/python3.12/site-packages/allure_behave/listener.py", line 97, in stop_test
self.stop_scenario(context['scenario'])
File "/Users/marcocarosi/Code/miniconda3/envs/phoenix3/lib/python3.12/site-packages/allure_behave/listener.py", line 100, in stop_scenario
should_run = (scenario.should_run_with_tags(self.behave_config.tags) and  # original
              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/Users/marcocarosi/Code/miniconda3/envs/phoenix3/lib/python3.12/site-packages/behave/model_core.py", line 398, in should_run_with_tags
return tag_expression.check(self.effective_tags)
       ^^^^^^^^^^^^^^^^^^^^
AttributeError: 'str' object has no attribute 'check'

That is a blocker for whoever wants to use allure with behave.

Checklist

Fix for issue #771: with behave v1.2.7.dev5 an error causes the allure_behave report formatter to crash
@ercaronte
Copy link
Contributor Author

ercaronte commented Mar 5, 2024

Seeing the failing CI tests. There is an API change in the behave Configuration class made on 1.2.7dev1 release.
Will see if I can made a fix so that the allure-behave listener supports <=1.2.6 versions as well as the new 1.2.7dev

There is an API change in the behave Configuration class from 1.2.6 to 1.2.7dev5.
With this fix the listener class should work in all cases.
@ercaronte
Copy link
Contributor Author

ercaronte commented Mar 5, 2024

Added a check on the config class attributes.
Maybe not very elegant, but now all tests pass with all non-dev versions of behave.

Not sure who to ask for a review.
@skhomuti or @delatrie, maybe you could help?

@delatrie
Copy link
Contributor

Hi, @ercaronte ! Thank you for you PR.

We usually only support generally available officially released versions. However, in the case of Behave, making an exception is reasonable since 1.2.7 pre-release versions are somewhat popular, and an alternative is to use six-year-old version 1.2.6.

That said, I suggest to change the version check to be more expressive by utilizing packaging.version:

import behave
from packaging import version

# ...

if version.parse(behave.__version__) > version.parse("1.2.6"):
    # new code
else:
    # old code

The check might be slightly optimized by evaluating at the module level:

# ...

BEHAVE_1_2_7_OR_GREATER = version.parse(behave.__version__) > version.parse("1.2.6")

# ...

if BEHAVE_1_2_7_OR_GREATER:
    # ...

Copy link
Contributor

@delatrie delatrie left a comment

Choose a reason for hiding this comment

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

Pls, see my comment above

Changing version check to be more expressive by utilizing packaging.version
@ercaronte
Copy link
Contributor Author

Hi @delatrie , thank you for your good feedback.
I like the proposed changes, they are definitely more expressive in view of future code maintenance changes.
I preferred the more optimised version at module level B-)

I tested locally on 1.2.7dev5, and for previous versions the CI tests are passing just fine.

@ercaronte ercaronte requested a review from delatrie March 12, 2024 12:25
Copy link
Contributor

@delatrie delatrie left a comment

Choose a reason for hiding this comment

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

Please consider one more change (see my other comment).

allure-behave/src/listener.py Outdated Show resolved Hide resolved
Preferring boolean and versus logical &

Co-authored-by: Maxim <[email protected]>
@ercaronte ercaronte requested a review from delatrie March 13, 2024 09:21
Copy link
Contributor

@delatrie delatrie left a comment

Choose a reason for hiding this comment

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

Let's make one more (final, I hope) change.

Currently, we can't test against 1.2.7.dev.5 because our ad-hoc runner that collects test results in memory depends on behave.matchers.step_matcher and behave.matchers.current_matcher, which got removed in Behave 1.2.7.dev.4. We don't need these, as our test samples don't use those functions.

We can safely remove the following lines in tests/allure_behave/behave_runner.py:

Line 77: "step_matcher":     matchers.step_matcher,
Line 83: default_matcher = matchers.current_matcher
Line 87: matchers.current_matcher = default_matcher

That should fix testing locally. As soon as Behave 1.2.7 is released on PyPI, we'll also include testing against both 1.2.6 and 1.2.7 in our CI. But for now, let's just fix local tests and merge.

…ehave.matchers.current_matcher, which got removed in Behave 1.2.7.dev.4.

In this way, the allure-behave test suite works for Behave 1.2.6 as well as with the latest 1.2.7dev5
@ercaronte
Copy link
Contributor Author

ercaronte commented Mar 13, 2024

Made the suggested changes, and run the 55 tests of tests/allure_behave with Behave 1.2.6 and with 1.2.7dev5.
They are passing just fine with both versions.

Of course, without this change with Behave 1.2.7dev5 all tests could not even start because of the matchers API change from 1.2.7dev4.

Good suggestion!

@ercaronte ercaronte requested a review from delatrie March 13, 2024 21:00
@delatrie
Copy link
Contributor

Nice job! Thank you for your time!

@delatrie delatrie merged commit d759bc5 into allure-framework:master Mar 14, 2024
12 checks passed
IvanBuruyane pushed a commit to IvanBuruyane/allure-python that referenced this pull request Mar 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants