You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
Add test cases to validate functionality of PUBLIC_URL and relative homepage #1516
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).
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
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?
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
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).
Activity
[-]Add test cases to validate functionality of PUBLIC_URL and `.` homepage[/-][+]Add test cases to validate functionality of PUBLIC_URL and relative homepage[/+]EnoahNetzach commentedon Feb 10, 2017
The test runner would have to
"homepage"fieldfor 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?
Timer commentedon Feb 10, 2017
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.
EnoahNetzach commentedon Feb 10, 2017
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
sedwould mostly work (?)Which are the test cases, and the expected outputs?
For the
PUBLIC_URLcase these are:https://www.example.org/crashould remain the same/crashould remain the sameFor the
"homepage"case:https://www.example.org/cra->/cra/crashould remain the same".": should be a relative pathAm I right?
Timer commentedon Feb 10, 2017
Correct @EnoahNetzach -- but we should also check the
PUBLIC_URLcases when the"homepage"is set opposite of what we're testing to make sure thatPUBLIC_URLoverrides 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"/staticand'/static(to make sure it was done everywhere and not just in one case that our test matched).Timer commentedon Feb 10, 2017
Are you giving this a whirl, @EnoahNetzach? Or should I try to pull something together?
Timer commentedon Feb 10, 2017
Started #1519. Feel free to contribute.
EnoahNetzach commentedon Feb 10, 2017
@Timer you go, I'll watch and judge 😛
Timer commentedon Feb 10, 2017
I'd love a review on #1519 @EnoahNetzach.
EnoahNetzach commentedon Feb 10, 2017
@Timer tomorrow I'll be more than happy
gaearon commentedon Mar 7, 2017
What's left here?
Timer commentedon Mar 7, 2017
The bash implementation isn't preferable, we would probably benefit more from a more comprehensive entry in kitchensink that uses jest.
gaearon commentedon Jan 8, 2018
Closing as stale.