Skip to content

Handle clients-tests requires section #48

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.

Sign up for GitHub

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

Merged
merged 3 commits into from
May 31, 2024
Merged

Handle clients-tests requires section #48

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
cd2c94f
Handle clients-tests requires section
pquentin May 30, 2024
0e542e4
Merge branch 'main' into requires-serverless
pquentin May 30, 2024
545a1cc
Mention `requires` section in comment
pquentin May 31, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion test_elasticsearch_serverless/test_async/test_server/test_rest_api_spec.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,9 @@ async def run_do(self, action):
headers.pop("Authorization")

method, args = list(action.items())[0]
args["headers"] = headers

if headers:
args["headers"] = headers

# locate api endpoint
for m in method.split("."):
Expand Down
9 changes: 7 additions & 2 deletions test_elasticsearch_serverless/test_server/test_rest_api_spec.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -568,20 +568,25 @@ def remove_implicit_resolver(cls, tag_to_remove):
# Each file may have a "test" named 'setup' or 'teardown',
Copy link
Contributor

Choose a reason for hiding this comment

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

This comment should also mention the requires section.

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks, addressed in 545a1cc (#48)

# these sets of steps should be run at the beginning and end
# of every other test within the file so we do one pass to capture those.
setup_steps = teardown_steps = None
requires = setup_steps = teardown_steps = None
test_numbers_and_steps = []
test_number = 0

for yaml_test in yaml_tests:
test_name, test_step = yaml_test.popitem()
if test_name == "setup":
if test_name == "requires":
requires = test_step
elif test_name == "setup":
setup_steps = test_step
elif test_name == "teardown":
teardown_steps = test_step
else:
test_numbers_and_steps.append((test_number, test_step))
test_number += 1

if not requires["serverless"]:
Copy link
Contributor

Choose a reason for hiding this comment

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

Can a test not have a requires section, or have it, but not have a serverless key in it? Not sure if that is possible, but I think it would make sense to protect against a crash here.

Copy link
Member Author

Choose a reason for hiding this comment

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

It's not possible. If it happens, I'd rather crash loudly than making a wrong choice (offensive programming vs. defensive programming). After all, we fully control the tests here.

continue

# Now we combine setup, teardown, and test_steps into
# a set of pytest.param() instances
for test_number, test_step in test_numbers_and_steps:
Expand Down
Loading