Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
THREESCALE-3927 Update Swagger to Open API 3.0 #3103
THREESCALE-3927 Update Swagger to Open API 3.0 #3103
Changes from 85 commits
6431713
917c5c6
f6c8f27
60f10bf
3474cd7
85291b4
0bd1736
320b866
19fcdd1
743ac7e
dd7698a
4d1f295
0ac7771
f25a892
cf33563
fd5658e
8590547
6ccf2a8
35cd821
871939d
f9a1d4d
d029760
37279ec
e289688
7af296e
2542482
f1462ae
60b6c15
07c9060
4b54cf2
8c22c5a
05929bb
2cd854b
dc6b27c
24af41b
5e4cdf5
b332e1a
0df1c24
b7658fc
4daad6d
11dc573
d69be1c
2ba20d4
dd88183
3015583
1a39f52
a0f20ee
2cc5d0b
dd43896
64aa8f0
8d92098
b7d8f01
47af16b
63010ac
1b82073
d0e290b
95fee48
be40a4c
ba46c19
45c28e9
7318724
3f11557
fad2c25
92810fa
3dc142a
12272ae
126fa27
b400392
ee90142
de22cf2
532deeb
81a49b9
1cbd7ea
b858333
eb07699
3bee640
3985412
9e39a67
ffa3f34
14ac7f0
1045e25
f2c9ac4
1f56654
78503bd
c420580
0700720
e6a7755
02ef45a
8200ed9
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems this is being exported only for tests. IMO this has became a private function now, so it shouldn't be exported. Tests shouldn't condition the code, they should adapt to the code. In this case, the tests to assert
autocompleteOAS3
logic should be included as tests for the 'fetching OpenAPI spec' context ofautocompleteRequestInterceptor
test, since that's the 'unit' being exported. What do you think?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That is right, and I agree with you that ideally the tests should adapt to the code, and not the other way around. On the other hand, when we code, we generally think about making the code testable (splitting codes in smaller units, adding dependency injection mechanism to allow for easier mocking of the dependencies, etc.).
In this case, I would really prefer to test the function
autocompleteOAS3
itself, and not the only "public" functionautocompleteRequestInterceptor
.I am not sure if there is another easy way to do this, without exporting the function (there are ways, but not too straightforward).
I am not sure I understand what you mean 🙃
@josemigallas what do you think?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I meant basically move
'should inject servers to the spec'
and'should autocomplete fields of OpenAPI spec with x-data-threescale-name property'
to line 100 after'should update the response interceptor'
. But OK.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jlledom like this? 8200ed9