Skip to content

Add test to show failure case for string factories.#53

Open
jalcine wants to merge 1 commit intobernardolins:masterfrom
jalcine:broken-support-for-non-string
Open

Add test to show failure case for string factories.#53
jalcine wants to merge 1 commit intobernardolins:masterfrom
jalcine:broken-support-for-non-string

Conversation

@jalcine
Copy link
Copy Markdown

@jalcine jalcine commented Mar 6, 2020

This adds a test case to show that using the response factory for non-map types causes a failure when attempting to build them. This makes it difficult to use this library for testing non-JSON like responses.

@coveralls
Copy link
Copy Markdown

Coverage Status

Coverage remained the same at 95.789% when pulling d744f8e on jalcine:broken-support-for-non-string into ffbfb3e on bernardolins:master.

1 similar comment
@coveralls
Copy link
Copy Markdown

Coverage Status

Coverage remained the same at 95.789% when pulling d744f8e on jalcine:broken-support-for-non-string into ffbfb3e on bernardolins:master.

@bernardolins
Copy link
Copy Markdown
Owner

Hi @jalcine, thanks for your contribution!

I think you want to make it clear that response factories only work with maps or strings that are in JSON format, right? This test you added will never pass, unless factories start to support plain text strings. Would you mind to change it to check if an error is raised so the test passes? I think something like this will do the work:

  test_with_server "basic factory usage with text" do
    assert_raise FakeServer.Error, fn ->
      route("/plain", MyResponseFactory.build(:plain_text))
    end
  end

Btw you can test plain text responses by using a Response structure like this:

  test_with_server "basic factory usage with text" do
    route "/plain", FakeServer.Response.ok!("Hello World", %{"Content-Type" => "text/plain"})
    response = HTTPoison.get!(FakeServer.address() <> "/plain")
    assert response.body == "Hello World"
  end

Please let me know if you have any problem using FakeServer!

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.

3 participants