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.

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

Jump to bottom

File uploads tests #101

Merged
merged 5 commits into from
Feb 7, 2020
Merged

File uploads tests #101

merged 5 commits into from
Feb 7, 2020

Conversation

macisamuele
Copy link
Collaborator

This PR has 2 main goals

  1. endpoint that are not multipart and that have file parameter should be consider unsafe
  2. add integration tests around the FileAPI
    Fixes: Verify correctness of endpoints with file parameters #99

In order to be sure that the "backend" will receive valid informations I've defined a Dispatcher that will returns a response containing details of the body:

  • is it RFC2387 compatible?
  • what is the boundary?
  • what are the details of the different body parts?

@macisamuele macisamuele self-assigned this Feb 6, 2020
@macisamuele macisamuele added bug Something isn't working enhancement New feature or request labels Feb 6, 2020
@macisamuele macisamuele added this to the 1.4.0 milestone Feb 6, 2020
@macisamuele
Copy link
Collaborator Author

macisamuele commented Feb 6, 2020 •
edited

@cortinico I'm going to publish a new iteration.
The reason for this are:

  • simplifying a bit the logic for extracting details of the request body
  • moving it into extension functions of PostFileEndpointTest instead of polluting MockServerApiRule (it was not really a great idea)
  • removing not needed objects on the specs

GitHub UI is not great for rebases (at least not in this case) 😢
SHA 876bec3 is the iteration mentioned in this comment, and so this is now fully ready to be reviewed

cortinico
cortinico requested changes Feb 6, 2020
View reviewed changes
samples/junit-tests/src/test/java/com/yelp/codegen/generatecodesamples/PostFileEndpointTest.kt Outdated Show resolved Hide resolved
samples/junit-tests/src/test/java/com/yelp/codegen/generatecodesamples/PostFileEndpointTest.kt Outdated Show resolved Hide resolved
samples/junit-tests/src/test/java/com/yelp/codegen/generatecodesamples/PostFileEndpointTest.kt Outdated Show resolved Hide resolved
samples/junit-tests/src/test/java/com/yelp/codegen/generatecodesamples/PostFileEndpointTest.kt Outdated Show resolved Hide resolved
samples/junit-tests/src/test/java/com/yelp/codegen/generatecodesamples/PostFileEndpointTest.kt Outdated Show resolved Hide resolved
samples/junit-tests/src/test/java/com/yelp/codegen/generatecodesamples/PostFileEndpointTest.kt Outdated Show resolved Hide resolved
samples/junit-tests/src/test/java/com/yelp/codegen/generatecodesamples/PostFileEndpointTest.kt Outdated Show resolved Hide resolved
samples/junit-tests/src/test/java/com/yelp/codegen/generatecodesamples/PostFileEndpointTest.kt Outdated Show resolved Hide resolved
macisamuele
macisamuele commented Feb 6, 2020
View reviewed changes
Copy link
Collaborator Author

@macisamuele macisamuele left a comment

Choose a reason for hiding this comment

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

Thanks for helping me be more aware of kotlin features!

Addressed the majority of comments and left few comments back

samples/junit-tests/src/test/java/com/yelp/codegen/generatecodesamples/PostFileEndpointTest.kt Outdated Show resolved Hide resolved
samples/junit-tests/src/test/java/com/yelp/codegen/generatecodesamples/PostFileEndpointTest.kt Outdated Show resolved Hide resolved
samples/junit-tests/src/test/java/com/yelp/codegen/generatecodesamples/PostFileEndpointTest.kt Outdated Show resolved Hide resolved
samples/junit-tests/src/test/java/com/yelp/codegen/generatecodesamples/PostFileEndpointTest.kt Outdated Show resolved Hide resolved
@macisamuele
PR Feedbacks: Remove usage of iterator and use a more functional appr…
e15d240 
…oach

Use assert* instead of if (...) fail(...)
cortinico
cortinico approved these changes Feb 7, 2020
View reviewed changes
Copy link
Collaborator

@cortinico cortinico left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@macisamuele macisamuele merged commit d689512 into Yelp:master Feb 7, 2020
@macisamuele macisamuele deleted the maci-fix-99-file-uploads branch February 7, 2020 15:26
@macisamuele macisamuele mentioned this pull request Feb 7, 2020
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@cortinico cortinico cortinico approved these changes

Assignees

@macisamuele macisamuele

Labels
bug Something isn't working enhancement New feature or request
Projects
None yet
Milestone
1.4.0
Development

Successfully merging this pull request may close these issues.

Verify correctness of endpoints with file parameters
2 participants
@macisamuele @cortinico