Skip to content

Commit db49df7

Browse files
jenisysjenisys
authored
Fix double-parenthesis in string representation (#95)
* FIXED #94 Not.to_string() conversion leads to double-parenthesis IMPLEMENTED FIX FOR: * [x] java * [x] javascript * [x] python * FIX CI-BUILD: Missing dependency PyYAML * Python 2.7 related issues (pytest, pathlib related) * FIX: tox to test Python 2.7 usecase locally UPDATE: Dependencies * setup.py * py.requirements/*.txt -- testing and development related * FIX CI BUILD: prettier formatting issue * FIXES #94 for Go programming language * notExpr.ToString() * ADDED: Evaluatable.isBinaryOperator() (used by: notExpr) REASON: Was faster than providing an instance-of implementation * FIXES #94 for Ruby programming language * Not.to_s() -- to_string conversion * FIXES #94 for Perl programming language * go: Provide IsBinaryOperator function * Remove IsBinaryOperator() method in Evaluatable interface * Provide IsBinaryOperator() function instead that encapsulates the is-instance-of And/Or expression. * perl: Use isa operator instead of isBinaryOperator method * Remove Node::isBinaryOperator() method and implementations * go: Use "isBinaryOperator" (naming style change) * Use "isBinaryOperator()" (was: "IsBinaryOperator") to comply w/ Go naming conventions and avoid to expose the this helper function to the public API of this package. * UPDATED: CHANGELOG for pull #95 * Fixes: #94 * Implements: #18 * CI workflow: test-python -- Remove Python 2.7 * REASON: Github Actions support for Python 2.7 was removed (in 2023-06) * Try to use pypy-2.7 instead (until Python 2.7 support is dropped) * CI workflow: test-python -- Remove Python 2.7 * REASON: Github Actions support for Python 2.7 was removed (in 2023-06) * Try to use pypy-2.7 instead (until Python 2.7 support is dropped)
1 parent f888f8d commit db49df7

File tree

22 files changed

+274
-24
lines changed

22 files changed

+274
-24
lines changed

‎.github/workflows/test-python.yml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ jobs:
2020
matrix:
2121
os:
2222
- ubuntu-latest
23-
python-version: [ '3.x', '2.x', 'pypy-3.8', 'pypy-2.7' ]
23+
python-version: [ "3.x", "pypy-3.10", "pypy-2.7" ]
2424
# DISABLED: python-version: [ '3.x', '2.x' ]
2525
# include:
2626
# - os: macos-latest

‎CHANGELOG.md

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,18 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
77

88
## [Unreleased]
99

10+
### Fixed
11+
12+
- [All] `Not.to_string()` conversion has unneeded double-parenthesis if binary operator is contained
13+
([#94](https://github.com/cucumber/tag-expressions/issues/94)
14+
by [jenisys](https://github.com/jenisys))
15+
16+
### Added
17+
18+
- [Python] Make tests pass against shared test data (except: backslash-escaped)
19+
([#18](https://github.com/cucumber/tag-expressions/issues/18)
20+
by [jenisys](https://github.com/jenisys))
21+
1022
## [5.0.1] - 2023-01-03
1123
### Fixed
1224
- [Java] Fix scm and project urls

‎go/parser.go

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -220,6 +220,12 @@ func (a *andExpr) ToString() string {
220220
return fmt.Sprintf("( %s and %s )", a.leftExpr.ToString(), a.rightExpr.ToString())
221221
}
222222

223+
func isBinaryOperator(e Evaluatable) bool {
224+
_, isBinaryAnd := e.(*andExpr)
225+
_, isBinaryOr := e.(*orExpr)
226+
return isBinaryAnd || isBinaryOr
227+
}
228+
223229
type notExpr struct {
224230
expr Evaluatable
225231
}
@@ -229,6 +235,10 @@ func (n *notExpr) Evaluate(variables []string) bool {
229235
}
230236

231237
func (n *notExpr) ToString() string {
238+
if isBinaryOperator(n.expr) {
239+
// -- HINT: Binary Operators already have already '( ... )'.
240+
return fmt.Sprintf("not %s", n.expr.ToString())
241+
}
232242
return fmt.Sprintf("not ( %s )", n.expr.ToString())
233243
}
234244

‎java/src/main/java/io/cucumber/tagexpressions/TagExpressionParser.java

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -251,6 +251,10 @@ public boolean evaluate(List<String> variables) {
251251

252252
@Override
253253
public String toString() {
254+
if (And.class.isInstance(expr) || Or.class.isInstance(expr)) {
255+
// -- HINT: Binary Operators already have already ' ( ... ) '.
256+
return "not " + expr.toString();
257+
}
254258
return "not ( " + expr.toString() + " )";
255259
}
256260
}

‎javascript/src/index.ts

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -214,6 +214,11 @@ class Not implements Node {
214214
}
215215

216216
public toString() {
217+
if (this.expr instanceof And || this.expr instanceof Or) {
218+
// -- HINT: Binary Operators already have already '( ... )'.
219+
return 'not ' + this.expr.toString()
220+
}
221+
// -- OTHERWISE:
217222
return 'not ( ' + this.expr.toString() + ' )'
218223
}
219224
}

‎perl/lib/Cucumber/TagExpressions/Node.pm

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -205,7 +205,11 @@ The wrapped node class instance for which to negate the result.
205205

206206
sub stringify {
207207
my ( $self ) = @_;
208-
208+
if ($self->expression->isa('Cucumber::TagExpressions::AndNode') ||
209+
$self->expression->isa('Cucumber::TagExpressions::OrNode')) {
210+
# -- HINT: Binary Operators already have already '( ... )'.
211+
return 'not ' . $self->expression->stringify;
212+
}
209213
return 'not ( ' . $self->expression->stringify . ' )';
210214
}
211215
}

‎python/.envrc

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,6 @@
1919
# -- USE OPTIONAL PARTS (if exist/enabled):
2020
dotenv_if_exists .env
2121
source_env_if_exists .envrc.use_venv
22-
source_env_if_exists .envrc.use_pep0582
2322

2423
# -- SETUP-PYTHON: Prepend ${HERE} to PYTHONPATH (as PRIMARY search path)
2524
# SIMILAR TO: export PYTHONPATH="${HERE}:${PYTHONPATH}"

‎python/cucumber_tag_expressions/model.py

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -144,7 +144,11 @@ def evaluate(self, values):
144144
return not self.term.evaluate(values_)
145145

146146
def __str__(self):
147-
return "not ( %s )" % self.term
147+
schema = "not ( {0} )"
148+
if isinstance(self.term, (And, Or)):
149+
# -- REASON: Binary operators have parenthesis already.
150+
schema = "not {0}"
151+
return schema.format(self.term)
148152

149153
def __repr__(self):
150154
return "Not(%r)" % self.term

‎python/py.requirements/develop.txt

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,10 @@ virtualenv >=20.4.5; python_version >= '3.0'
1717
# FORMER: bumpversion >= 0.4.0
1818
bump2version >= 0.5.6
1919

20+
# -- RELEASE MANAGEMENT: Push package to pypi.
21+
twine >= 1.13.0
22+
23+
# -- DEVELOPMENT SUPPORT:
2024
# -- PYTHON2/3 COMPATIBILITY: pypa/modernize
2125
# python-futurize
2226
modernize >= 0.5
@@ -25,11 +29,14 @@ modernize >= 0.5
2529
# PREPARED: prospector >= 0.12.7
2630
pylint >= 1.7
2731
bandit >= 1.4; python_version >= '3.7'
32+
ruff
2833

2934
# -- LOCAL CI (for Python):
3035
# Test with different Python versions.
3136
# SEE: https://tox.wiki/
32-
tox >= 2.9
37+
tox >=2.9,<4.0
38+
tox >=2.9,<4.0 # -- HINT: tox >= 4.0 has breaking changes.
39+
virtualenv < 20.22.0 # -- SUPPORT FOR: Python 2.7, Python <= 3.6
3340

3441
# -- REQUIRES: testing, docs, invoke-task requirements
3542
# PREPARED: -r docs.txt

‎python/py.requirements/testing.txt

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,15 @@
22
# PYTHON PACKAGE REQUIREMENTS FOR: For testing only
33
# ============================================================================
44

5-
pytest >= 3.2
6-
pytest-html >= 1.19
5+
# -- TESTING: Unit tests and behave self-tests.
6+
pytest < 5.0; python_version < '3.0' # pytest >= 4.2
7+
pytest >= 5.0; python_version >= '3.0'
8+
9+
pytest-html >= 1.19.0,<2.0; python_version < '3.0'
10+
pytest-html >= 2.0; python_version >= '3.0'
11+
12+
PyYAML >= 5.4.1
13+
pathlib; python_version <= '3.4'
714

815
# -- DEVELOPMENT SUPPORT: Testing
916
coverage >= 4.2

‎python/setup.py

Lines changed: 14 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -68,20 +68,27 @@ def find_packages_by_root_package(where):
6868
"enum34; python_version < '3.4'"
6969
],
7070
tests_require=[
71-
"pytest >= 3.2",
72-
"pytest-html >= 1.19.0",
71+
"pytest < 5.0; python_version < '3.0'",
72+
"pytest >= 5.0; python_version >= '3.0'",
73+
"pytest-html >= 1.19.0,<2.0; python_version < '3.0'",
74+
"pytest-html >= 2.0; python_version >= '3.0'",
75+
"PyYAML >= 5.4.1",
76+
"pathlib; python_version <= '3.4'",
7377
],
7478
extras_require={
7579
# PREPARED: 'docs': ["sphinx>=1.5"],
7680
"develop": [
7781
"coverage",
78-
"pytest >= 3.2",
79-
"pytest-html >= 1.19.0",
80-
"tox >= 2.9",
82+
"pytest < 5.0; python_version < '3.0'",
83+
"pytest >= 5.0; python_version >= '3.0'",
84+
"pytest-html >= 1.19.0,<2.0; python_version < '3.0'",
85+
"pytest-html >= 2.0; python_version >= '3.0'",
86+
"tox >=2.9,<4.0",
8187
"pylint",
88+
"ruff",
8289
# -- INVOKE SUPPORT:
83-
"invoke >= 1.4.1",
84-
"six >= 1.15.0",
90+
"invoke >= 1.7.3",
91+
"six >= 1.16.0",
8592
"path >= 13.1.0; python_version >= '3.5'",
8693
"path.py >= 11.5.0; python_version < '3.5'",
8794
# PYTHON2 BACKPORTS:

‎python/tasks/py.requirements.txt

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,8 +8,8 @@
88
# * http://www.pip-installer.org/
99
# ============================================================================
1010

11-
invoke >= 1.4.1
12-
six >= 1.15.0
11+
invoke >= 1.7.3
12+
six >= 1.16.0
1313

1414
# -- HINT: path.py => path (python-install-package was renamed for python3)
1515
path >= 13.1.0; python_version >= '3.5'

‎python/tests/data/README.md

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
This director contains "data-driven" tests from the "../../../testdata/*.yaml" directory.
2+
3+
DATA FILES:
4+
5+
* errors.yml
6+
* evaluations.yml
7+
* parsing.yml

‎python/tests/data/__init__.py

Whitespace-only changes.

‎python/tests/data/test_errors.py

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,53 @@
1+
# -*- coding: UTF-8 -*-
2+
from __future__ import absolute_import, print_function
3+
from collections import namedtuple
4+
from pathlib import Path
5+
6+
from cucumber_tag_expressions.parser import TagExpressionParser, TagExpressionError
7+
import pytest
8+
import yaml
9+
10+
11+
# -----------------------------------------------------------------------------
12+
# DATA-FILE CONSTANTS:
13+
# -----------------------------------------------------------------------------
14+
HERE = Path(__file__).parent.absolute()
15+
TESTDATA_DIRECTORY = HERE/"../../../testdata"
16+
TESTDATA_FILE = TESTDATA_DIRECTORY/"errors.yml"
17+
18+
19+
# -----------------------------------------------------------------------------
20+
# DATA-FILE DRIVEN TEST SUPPORT:
21+
# -----------------------------------------------------------------------------
22+
# - expression: '@a @b or'
23+
# error: 'Tag expression "@a @b or" could not be parsed because of syntax error: Expected operator.'
24+
DTestData4Error = namedtuple("DTestData4Error", ("expression", "error"))
25+
26+
def read_testdata(data_filename):
27+
testdata_items = []
28+
with open(str(data_filename)) as f:
29+
for item in yaml.safe_load(f):
30+
assert isinstance(item, dict)
31+
data_item = DTestData4Error(item["expression"], item["error"])
32+
testdata_items.append(data_item)
33+
return testdata_items
34+
35+
36+
37+
# -----------------------------------------------------------------------------
38+
# TEST SUITE:
39+
# -----------------------------------------------------------------------------
40+
this_testdata = read_testdata(TESTDATA_FILE)
41+
42+
@pytest.mark.skip(reason="TOO MANY DIFFERENCES: Error message here are more specific (IMHO)")
43+
@pytest.mark.parametrize("expression, error", this_testdata)
44+
def test_errors_with_datafile(expression, error):
45+
if "\\" in expression:
46+
pytest.skip("BACKSLASH-ESCAPING: Not supported yet")
47+
48+
with pytest.raises(TagExpressionError) as exc_info:
49+
_ = TagExpressionParser().parse(expression)
50+
51+
exc_text = exc_info.exconly()
52+
print(exc_text)
53+
assert error in exc_text

‎python/tests/data/test_evaluations.py

Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,63 @@
1+
# -*- coding: UTF-8 -*-
2+
from __future__ import absolute_import, print_function
3+
from collections import namedtuple
4+
from pathlib import Path
5+
6+
from cucumber_tag_expressions.parser import TagExpressionParser
7+
import pytest
8+
import yaml
9+
10+
11+
# -----------------------------------------------------------------------------
12+
# DATA-FILE CONSTANTS:
13+
# -----------------------------------------------------------------------------
14+
HERE = Path(__file__).parent.absolute()
15+
TESTDATA_DIRECTORY = HERE/"../../../testdata"
16+
TESTDATA_FILE = TESTDATA_DIRECTORY/"evaluations.yml"
17+
18+
19+
# -----------------------------------------------------------------------------
20+
# DATA-FILE DRIVEN TEST SUPPORT:
21+
# -----------------------------------------------------------------------------
22+
# - expression: 'not x'
23+
# tests:
24+
# - variables: ['x']
25+
# result: false
26+
# - variables: ['y']
27+
# result: true
28+
DTestData4Evaluation = namedtuple("DTestData4Evaluation", ("expression", "tests"))
29+
DTestVarsAndResult = namedtuple("DTestVarsAndResult", ("variables", "result"))
30+
31+
def read_testdata(data_filename):
32+
testdata_items = []
33+
with open(str(data_filename)) as f:
34+
for item in yaml.safe_load(f):
35+
assert isinstance(item, dict)
36+
tests = []
37+
for test in item["tests"]:
38+
test_variables = test["variables"]
39+
test_result = test["result"]
40+
tests.append(DTestVarsAndResult(test_variables, test_result))
41+
data_item = DTestData4Evaluation(item["expression"], tests)
42+
testdata_items.append(data_item)
43+
return testdata_items
44+
45+
46+
47+
# -----------------------------------------------------------------------------
48+
# TEST SUITE:
49+
# -----------------------------------------------------------------------------
50+
this_testdata = read_testdata(TESTDATA_FILE)
51+
52+
@pytest.mark.parametrize("expression, tests", this_testdata)
53+
def test_parsing_with_datafile(expression, tests):
54+
if "\\" in expression:
55+
pytest.skip("BACKSLASH-ESCAPING: Not supported yet")
56+
57+
print("expression := {0}".format(expression))
58+
tag_expression = TagExpressionParser().parse(expression)
59+
for test_data in tests:
60+
print("test.variables= {0}".format(test_data.variables))
61+
print("test.result = {0}".format(test_data.result))
62+
actual_result = tag_expression.evaluate(test_data.variables)
63+
assert actual_result == test_data.result

‎python/tests/data/test_parsing.py

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,47 @@
1+
# -*- coding: UTF-8 -*-
2+
from __future__ import absolute_import, print_function
3+
from collections import namedtuple
4+
from pathlib import Path
5+
6+
from cucumber_tag_expressions.parser import TagExpressionParser
7+
import pytest
8+
import yaml
9+
10+
11+
# -----------------------------------------------------------------------------
12+
# DATA-FILE CONSTANTS:
13+
# -----------------------------------------------------------------------------
14+
HERE = Path(__file__).parent.absolute()
15+
TESTDATA_DIRECTORY = HERE/"../../../testdata"
16+
TESTDATA_FILE = TESTDATA_DIRECTORY/"parsing.yml"
17+
18+
19+
# -----------------------------------------------------------------------------
20+
# DATA-FILE DRIVEN TEST SUPPORT:
21+
# -----------------------------------------------------------------------------
22+
DTestData4Parsing = namedtuple("DTestData4Parsing", ("expression", "formatted"))
23+
24+
def read_testdata(data_filename):
25+
testdata_items = []
26+
with open(str(data_filename)) as f:
27+
for item in yaml.safe_load(f):
28+
assert isinstance(item, dict)
29+
data_item = DTestData4Parsing(item["expression"], item["formatted"])
30+
testdata_items.append(data_item)
31+
return testdata_items
32+
33+
34+
35+
# -----------------------------------------------------------------------------
36+
# TEST SUITE:
37+
# -----------------------------------------------------------------------------
38+
this_testdata = read_testdata(TESTDATA_FILE)
39+
40+
@pytest.mark.parametrize("expression, formatted", this_testdata)
41+
def test_parsing_with_datafile(expression, formatted):
42+
if "\\" in expression:
43+
pytest.skip("BACKSLASH-ESCAPING: Not supported yet")
44+
45+
tag_expression = TagExpressionParser().parse(expression)
46+
actual_text = str(tag_expression)
47+
assert actual_text == formatted

‎python/tests/unit/test_model.py

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -95,6 +95,14 @@ def test_evaluate1(self, expected, tags, case):
9595
expression = Not(Literal("a"))
9696
assert expression.evaluate(tags) == expected
9797

98+
# -- HINT: Not with binary operator was using double-parenthesis in the past
99+
@pytest.mark.parametrize("expected, expression", [
100+
("not ( a and b )", Not(And(Literal("a"), Literal("b"))) ),
101+
("not ( a or b )", Not(Or(Literal("a"), Literal("b"))) ),
102+
("( a and not ( b or c ) )", And(Literal("a"), Not(Or(Literal("b"), Literal("c")))) ),
103+
])
104+
def test_convert_to_string(self, expected, expression):
105+
assert expected == str(expression)
98106

99107
class TestTrueOperation(object):
100108
@pytest.mark.parametrize("expected, tags, case", [

‎python/tests/unit/test_parser.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -159,7 +159,7 @@ def test_parse__empty_is_always_true(self, text):
159159
("a or not b", "( a or not ( b ) )"),
160160
("not a and b", "( not ( a ) and b )"),
161161
("not a or b", "( not ( a ) or b )"),
162-
("not (a and b) or c", "( not ( ( a and b ) ) or c )"),
162+
("not (a and b) or c", "( not ( a and b ) or c )"),
163163
])
164164
def test_parse__ensure_precedence(self, text, expected):
165165
"""Ensures that the operation precedence is parsed correctly."""

0 commit comments

Comments
 (0)