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

Removing host from test configs #23122

Open
wants to merge 1 commit into
base: 5.x-dev
Choose a base branch
from
Open

Removing host from test configs #23122

wants to merge 1 commit into from

Conversation

snake14
Copy link
Contributor

@snake14 snake14 commented Mar 13, 2025

Description:

On Ubuntu 24.04, I found that while running ddev matomo:init:tests the building of the OmniFixture failed due to a Connection refused error. Once I removed the http_host from the tests configs, everything ran as expected.

Review

@snake14 snake14 added the Needs Review PRs that need a code review label Mar 13, 2025
@michalkleiner
Copy link
Contributor

Hi @snake14, was it a problem when the DDEV project name was different to matomo.ddev.site?

@michalkleiner
Copy link
Contributor

Testing on a Mac without the http_host config seems to work ok, so it might be safe to remove this. UI tests ran as expected.

@snake14
Copy link
Contributor Author

snake14 commented Mar 13, 2025

Hi @snake14, was it a problem when the DDEV project name was different to matomo.ddev.site?

Hi @michalkleiner . Yes. When running 5x_dev.ddev.site, the only config I set under tests is the request_uri. If I set the http_host tests fail during initialisation.

@sgiehl
Copy link
Member

sgiehl commented Mar 17, 2025

I guess the best solution would be to set the hostname to the one configured in ddev config. That should in best case also happen for this config: https://github.com/matomo-org/matomo/blob/5.x-dev/.ddev/initial-config/config.js
Otherwise UI tests might not work.

@snake14
Copy link
Contributor Author

snake14 commented Mar 17, 2025

I guess the best solution would be to set the hostname to the one configured in ddev config. That should in best case also happen for this config: https://github.com/matomo-org/matomo/blob/5.x-dev/.ddev/initial-config/config.js Otherwise UI tests might not work.

@sgiehl I'm not sure I follow. This PR is because setting the tests http_host to the same host as the ddev config isn't working for Ubuntu. The tests aren't able to connect. They only worked as expected when I removed the http_host like I did in this PR.

@sgiehl
Copy link
Member

sgiehl commented Mar 18, 2025

@snake14 I thought you were using 5x_dev.ddev.site in your ddev name config for setup, but the test setup always uses matomo.ddev.site. So imho we should adjust the test setup to use whatever is configured as name for ddev. Or are you using mutliple hosts?
In any case UI tests would currently also not work, due to the fixed config for matomo.ddev.site

@michalkleiner
Copy link
Contributor

@snake14 and did you run the tests from inside the DDEV environment? They should be run via ddev matomo:console tests:run-ui....

I agree with Stefan that we could update all the hosts configs to be in line with the project name, but that is more related to our internal setup rather than the config here in the open source project repo where we anticipate people will run it on the default matomo.ddev.site domain.

@michalkleiner
Copy link
Contributor

@snake14 if you could confirm the UI tests work with the http_host config in place when running them inside the ddev containers, then we can close this PR.

@sgiehl
Copy link
Member

sgiehl commented Mar 18, 2025

@michalkleiner I have it running under a different hostname and UI tests don't run with the config created by the init.

@michalkleiner
Copy link
Contributor

michalkleiner commented Mar 18, 2025

Interesting, @sgiehl, thanks for confirming that. I never had a problem with that, but that could be OS-dependent or docker provider-dependent thing.

Anyway, the default configuration in the core repo is meant to work seamlessly for matomo.ddev.site, and once developers start changing things like the project name, things may start playing up. That is more a thing for the documentation we need to update on developer docs anyway.

If things work for matomo.ddev.site with the default config when running the UI tests from inside the ddev containers using ddev matomo:console tests:run-ui... then I would suggest we close the issue and adjust the internal start script to update the config values based on the project name.

@snake14
Copy link
Contributor Author

snake14 commented Mar 18, 2025

@sgiehl @michalkleiner I was using 5x_dev.ddev.site, but have switched to using the stock core config, which is matomo.ddev.site. That's how I ran into this issue. I started over by cloning the core repo into a new directory. As mentioned in the description, when the http_host was set, the ddev matomo:init:tests command would result in an error. So, I wasn't even able to make it as far as running UI tests until that config was removed.

Edit: I just tested, and it does appear to run the UI tests alright with or without the http_host set. So, I guess we could leave it in if the issue with the OmniFixture build command is fixed so that it works for Ubuntu when the http_host is set.

@michalkleiner
Copy link
Contributor

What error does the OmniFixture creation produces, @snake14? I just ran default core config and had no issues on a Mac, it's strange that it would behave differently on other OS's. We can discuss internally perhaps.

@snake14
Copy link
Contributor Author

snake14 commented Mar 18, 2025

@michalkleiner It appears to be a connection refused error from the python script executed by Fixture::executeLogImporter. When the tests.http_host config is set, the script uses --url="http://matomo.ddev.site:80/tests/PHPUnit/proxy/" and when it's not set, --url="http://localhost:80/tests/PHPUnit/proxy/" is used. I ran ddev ssh and curl http://matomo.ddev.site:80/tests/PHPUnit/proxy/ to confirm that it cannot connect to that host. However, it works fine for localhost. Running curl https://matomo.ddev.site/tests/PHPUnit/proxy/ also works. So, maybe it's an Apache config issue within DDEV forcing HTTPS? However, you'd think that our Apache configs within the container would be the same.

Copy link
Contributor

This issue is in "needs review" but there has been no activity for 7 days. ping @matomo-org/core-reviewers

@github-actions github-actions bot added the Stale The label used by the Close Stale Issues action label Mar 26, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Needs Review PRs that need a code review Stale The label used by the Close Stale Issues action
Development

Successfully merging this pull request may close these issues.

3 participants