Skip to content
This repository has been archived by the owner on Apr 23, 2019. It is now read-only.

Allow localhost:9000 explicitly in example. #81

Open
wants to merge 2 commits into
base: 2.6.x
Choose a base branch
from

Conversation

wsargent
Copy link
Member

@marcospereira
Copy link
Member

Beware the tests are failing because they are using another port (not 9000).

@wsargent
Copy link
Member Author

I think this is failing because FakeRequest will render with HOST set to localhost with no port at all if not explicitly defined:

class FakeRequestFactory(requestFactory: RequestFactory) {
  def apply(): FakeRequest[AnyContentAsEmpty.type] = {
    apply(method = "GET", uri = "/", headers = FakeHeaders(Seq(HeaderNames.HOST -> "localhost")), body = AnyContentAsEmpty)
  }
}

https://github.com/playframework/playframework/blob/master/framework/src/play-test/src/main/scala/play/api/test/Fakes.scala#L174

whereas it should be using the default test port (or whatever it is overridden to in the application).

class FunctionalSpec extends PlaySpec with GuiceOneServerPerSuite {

  "Routes" should {
    "send 404 on a bad request" in  {
      route(app, FakeRequest(GET, "/boum").withHeaders(FakeHeaders(Seq(HeaderNames.HOST -> s"localhost:$port")))).map(status(_)) mustBe Some(NOT_FOUND)
    }

    "send 200 on a good request" in  {
      route(app, FakeRequest(GET, "/").withHeaders(FakeHeaders(Seq(HeaderNames.HOST -> s"localhost:$port")))).map(status(_)) mustBe Some(OK)
    }

  }
}

The problem is that GuiceOneAppPerSuite doesn't have a port. The best thing to do is use play.api.test.Helpers.testServerPort in that case.

@wsargent
Copy link
Member Author

Fix needs to change documentation as well. https://www.playframework.com/documentation/2.6.x/AllowedHostsFilter#Testing

Also note the original example was actually about 127.0.0.1 rather than localhost as it was running on Windows 10, so when the original user uncommented, the default did not work: playframework/play-java-starter-example#60

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants