Skip to content

Add test cases to validate functionality of PUBLIC_URL and relative homepage #1516

Closed

Description

@Timer

We recently added PUBLIC_URL support and official . (relative) homepage support.

I think it's very important to test these behaviors to ensure they're being properly applied.
PUBLIC_URL is already partially tested but could be improved, and there exists no test for relative homepage support.

#1519 is the first step of this, we need much deeper integration testing (with other assets, maybe include bootstrap).

Activity

  1. changed the title [-]Add test cases to validate functionality of PUBLIC_URL and `.` homepage[/-] [+]Add test cases to validate functionality of PUBLIC_URL and relative homepage[/+] on Feb 10, 2017
  2. EnoahNetzach commented on Feb 10, 2017

    @EnoahNetzach
    Contributor

    The test runner would have to

    • create a new project (the simplest one), or
    • create a single project and modify the "homepage" field

    for each test case.

    A way to test them could be that of compiling them in various cases, and look for usages in the code afterward (similar to what is done here.)

    This way could be really demanding for travis' CI timewise, though..
    Tests could be conducted either in "installs" or in "kitchensink", although the latter is currently the slowest (in node 4), or in a separate "e2e" test case altogether.

    Another open question is: should there be a difference between production and development, therefore tested?

  3. Timer commented on Feb 10, 2017

    @Timer
    ContributorAuthor

    That's exactly what I had in mind, so it's great to hear that you're on the same page @EnoahNetzach.

    I think testing this case warrants some extra CI time, but I believe it's something we only have to test on Node 6.

    Also, we removed the PUBLIC_URL effect in development so it remained the same as before. I want to test that it's not respected.

  4. EnoahNetzach commented on Feb 10, 2017

    @EnoahNetzach
    Contributor

    Probably the long execution of "kitchensink" in node 4 lets us add another job to the travis' matrix, dedicated to this (and maybe future related) problems.
    It would still finish before that long one.

    I'd try it creating a new project each time (similar to "installs"), and see how much it takes..

    For testing I think a simple sed would mostly work (?)

    Which are the test cases, and the expected outputs?

    For the PUBLIC_URL case these are:

    • full URL: https://www.example.org/cra should remain the same
    • URI: /cra should remain the same

    For the "homepage" case:

    • full URL: https://www.example.org/cra -> /cra
    • URI: /cra should remain the same
    • ".": should be a relative path

    Am I right?

  5. Timer commented on Feb 10, 2017

    @Timer
    ContributorAuthor

    Correct @EnoahNetzach -- but we should also check the PUBLIC_URL cases when the "homepage" is set opposite of what we're testing to make sure that PUBLIC_URL overrides it.

    Checking "." should be as simple as: grep -R -q "../../static/" build/ and checking the exit code. Others similarly. We should also check that some greps are false, such as "/static and '/static (to make sure it was done everywhere and not just in one case that our test matched).

  6. Timer commented on Feb 10, 2017

    @Timer
    ContributorAuthor

    Are you giving this a whirl, @EnoahNetzach? Or should I try to pull something together?

  7. Timer commented on Feb 10, 2017

    @Timer
    ContributorAuthor

    Started #1519. Feel free to contribute.

  8. EnoahNetzach commented on Feb 10, 2017

    @EnoahNetzach
    Contributor

    @Timer you go, I'll watch and judge 😛

  9. Timer commented on Feb 10, 2017

    @Timer
    ContributorAuthor

    I'd love a review on #1519 @EnoahNetzach.

  10. EnoahNetzach commented on Feb 10, 2017

    @EnoahNetzach
    Contributor

    @Timer tomorrow I'll be more than happy

  11. gaearon commented on Mar 7, 2017

    @gaearon
    Contributor

    What's left here?

  12. Timer commented on Mar 7, 2017

    @Timer
    ContributorAuthor

    The bash implementation isn't preferable, we would probably benefit more from a more comprehensive entry in kitchensink that uses jest.

  13. gaearon commented on Jan 8, 2018

    @gaearon
    Contributor

    Closing as stale.

  14. locked and limited conversation to collaborators on Jan 20, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions