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

Jump to bottom

travis tests all have the https check though it should be disabled #13271

Closed
diosmosis opened this issue Aug 10, 2018 · 2 comments
Closed

travis tests all have the https check though it should be disabled #13271

diosmosis opened this issue Aug 10, 2018 · 2 comments
Labels
c: Tests & QA For issues related to automated tests or making it easier to QA & test issues. not-in-changelog For issues or pull requests that should not be included in our release changelog on matomo.org.
Milestone
3.6.0

Comments

@diosmosis
Copy link
Member

For some reason now UI tests aren't skipping the HTTPS check in ControllerAdmin::notifyIfURLIsNotSecure() which causes most tests to fail since the HTTPS notification shows up.

I checked and the host on travis-ci is still localhost, but Url::isLocalHost(Url::getCurrentHost()) returns false for some reason. I'm not sure why. I tried w/o the strict in_array() call, but it didn't work.

I added a check for if (defined('PIWIK_TEST_MODE')) return; but that change was rejected.
@diosmosis diosmosis added c: Tests & QA For issues related to automated tests or making it easier to QA & test issues. not-in-changelog For issues or pull requests that should not be included in our release changelog on matomo.org. labels Aug 10, 2018
@diosmosis diosmosis added this to the 3.6.0 milestone Aug 10, 2018
@sgiehl
Copy link
Member

sgiehl commented Aug 10, 2018

It's not localhost but localhost:3000.
@diosmosis your debugging output somehow got trimmed. printing the host urlencoded shows localhost%3A3000. Not sure why the port is now included in _SERVER['HTTP_HOST']

@diosmosis
Copy link
Member Author

I see, that makes more sense now, thanks @sgiehl I'll put a fix in my other PR.

@diosmosis diosmosis mentioned this issue Aug 10, 2018
Merged
@GTVolk GTVolk mentioned this issue Jun 20, 2023
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
c: Tests & QA For issues related to automated tests or making it easier to QA & test issues. not-in-changelog For issues or pull requests that should not be included in our release changelog on matomo.org.
Projects
None yet
Milestone
3.6.0
Development

No branches or pull requests
2 participants
@diosmosis @sgiehl