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.

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

feat: add mocked http request tests #1395

Closed
wants to merge 1 commit into from

Conversation

drbh
Copy link
Collaborator

@drbh drbh commented Jan 3, 2024

This PR implements two http request tests, and adds helper functions to test JSON requests and expected outputs.

test server::tests::test_echo_inputs_when_skip_generation ... ok
test server::tests::test_return_json_error_on_empty_inputs ... ok

Notes:

  • this pr adds tower = "0.4.13" so we can use the tower::util::ServiceExt trait to allow oneshot requests in the tests without starting an HTTP server.
  • skip_generation has been added to GenerateParameters as a way to avoid making requests to the router.

@Narsil and @OlivierDehaene please let me know what you think of this approach and if there is a better way to implement tests (the longer term goal is to improve TGI's API and these tests may be helpful when extending the API)

Copy link
Collaborator

@Narsil Narsil left a comment

Choose a reason for hiding this comment

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

Could you try not creating any surface just for tests ?

Tests are after all just for tests, they shouldn't ever require the code to change to accomodate for them.

Also adding new dependency is really a big thing imo, we should really refrain from doing so (especially since axum should provide everything we need already).

@drbh drbh closed this Jan 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants