Allow global variables #507
Conversation
* Fix the --browser option The problem was that webbrowser.open expects a URI and doesn't work with relative local paths. Solution: use pathlib to convert reporter.output_path to a URI using pathlib.Path(reporter.output_path).resolve().as_uri(). * Add a changelog entry
gabrieltron
force-pushed
the
global-variables
branch
4 times, most recently
from
f13ac53
to
1d9cabf
Compare
gabrieltron
force-pushed
the
global-variables
branch
2 times, most recently
from
26511c2
to
775c04a
Compare
There was a problem hiding this comment.
Awesome PR @gabrieltron! This was a super desired feature that we were trying to deliver for a long time. Thank you very much!
Also, pretty elegant solution 🌟
I left only one small change request, let me know what do you think :)
| @mark.context("when key exists in endpoint node") | ||
| @mark.it("should search endpoint node") | ||
| @mark.it("should return the value in endpoint node") | ||
| def test_should_return_list(self, spec_evaluator): |
There was a problem hiding this comment.
| def test_should_return_list(self, spec_evaluator): | |
| def test_should_return_list(self, mocker, spec_evaluator): |
| def test_should_return_list(self, spec_evaluator): | ||
| key = "requests" | ||
| expected_value = "value" | ||
| with mock.patch( |
There was a problem hiding this comment.
| with mock.patch( | |
| with mocker.patch( |
| @@ -1,4 +1,5 @@ | |||
| from pytest import fixture, mark, raises | |||
| from unittest import mock | |||
There was a problem hiding this comment.
We don't need to import mock from unittest because we use the pytest-mock library. It already provides a mocker fixture which is a thin-wrapper around the patching API provided by the mock package:
| from unittest import mock |
The two next suggestions show how to do it using mocker fixture.
There was a problem hiding this comment.
Yup! Fixed it. I forgot that the scope of a fixture is a function by default, so now I just used the existing mock from the fixture instead 😅
| @@ -0,0 +1,29 @@ | |||
| from pytest import mark | |||
There was a problem hiding this comment.
@gabrieltron can we have a test in which we have vars with the same name for both parent and child? To check that the inner value should be the one returned: the child's one.
For example:
endpoints:
- name: snippets-api
path: http://demo.scanapi.dev/api/v1/
headers:
Content-Type: application/json
vars:
my_var: one
requests:
- name: health
path: health?${my_var}
endpoints:
- name: endpoint
vars:
my_var: two
requests:
- name: another health
path: health?${my_var}Request one should be: GET http://demo.scanapi.dev/api/v1/health?one
Request two should be: GET http://demo.scanapi.dev/api/v1/health?two
There was a problem hiding this comment.
Nice, I hadn't thought of that scenario. I changed the method so it only propagates the new vars.
I also changed it so it only writes if the variable doesn't exist. On the downside, this implies that there's no way of update global variables. On the upside, it further protects the variables already declared in the parents. Do you think this is a good approach? Or should request nodes be able to update global variables?
gabrieltron
force-pushed
the
global-variables
branch
from
775c04a
to
31035fd
Compare
gabrieltron
force-pushed
the
global-variables
branch
from
31035fd
to
5bb3df8
Compare
|
Any news on that? I'd appreciate feedback, even if it's that we should hold this up for now hahaha |
Description
Allow global variables. When
spec_varsis updated after performing a call, instead of updating thespec_varsof just the endpoint node, recursively update all parent endpoint nodes. When evaluating an expression, instead of searching for the variable inspec_varsof only the request's endpoint node, search all parent request nodes recursively.Motivation behind this PR?
#265
What type of change is this?
Feature
Checklist
Issue
Closes #265