@peterhashair opened this Pull Request on November 10th 2021 Contributor

Description:

It's kind of working, the screenshot updated in the PR is because of some spark line like 1px different.Still 5-7 random failed on UI need investigate . some of the failed UI is related to stick table Head I made, the width is detected by browser is changing, I will update the test for that one.

Review

@peterhashair commented on November 11th 2021 Contributor

I think this is kind of working. coupe random fails
image

@peterhashair commented on November 11th 2021 Contributor

I guess this is ready for review, just 2 submodule image needs an update.
update UIs are all sparkline-related. Somehow the sparkline got 100px diff, although it looks the same.

I am not too sure about those. @sgiehl do you know why those $ not find is failing
https://github.com/matomo-org/matomo/runs/4175351607?check_suite_focus=true

1) Overlay should load correctly: Evaluation failed: ReferenceError: $ is not defined at __puppeteer_evaluation_script__:2:17 Url to reproduce: http://localhost/tests/PHPUnit/proxy/?module=Overlay&period=year&date=today&idSite=3#?l=http$3A$2F$2F127.0.0.1$2Ftests$2Fresources$2Foverlay-test-site-real$2F Screenshot of failure: /home/runner/work/matomo/matomo/tests/UI/processed-ui-screenshots/should_load_correctly_failure.png

@sgiehl commented on November 11th 2021 Member

Haven't had a closer look yet, but regarding

I am not too sure about those. @sgiehl do you know why those $ not find is failing
https://github.com/matomo-org/matomo/runs/4175351607?check_suite_focus=true

Those tests are trying to open a page on http://127.0.0.1/tests/resources/overlay-test-site-real/, which should be served by the configured webserver. Not sure if your implementation actually serves something using the IP or maybe creating that site fails for some reason here: https://github.com/matomo-org/matomo/blob/2404b24c3a52f04475b63f300269a129399882ec/tests/PHPUnit/Fixtures/UITestFixture.php#L282

@tsteur commented on November 11th 2021 Member

I haven't tested it but something that will need to be looked at is to make this work for our many plugins. With travis, a plugin only adds a .travis.yml and it works see eg https://github.com/matomo-org/plugin-QueuedTracking/blob/4.x-dev/.travis.yml . This travis.yml doesn't have much logic as otherwise developers might often need to change the plugin travis.yml and they wouldn't know what needs changing.

I don't know what this all means for the code in here. Effectively, a plugin should only have to create a workflow yml with ideally little logic to make the tests work.

I would assume that some of these scripts in the matomo-tests.yml will need to be moved to external scripts
image

Also I'm assuming that for plugins the directory structure will be different
image

Not sure if we want to cover this separately in a separate PR or what is best/easiest?

I'm mostly mentioning in case this means there will be bigger refactorings needed in the way it works.

@peterhashair commented on November 12th 2021 Contributor

@tsteur right, I was wondering why in bash it has so many variables set for plugin names, now I get it. Maybe I should convert all the run command into a bash file take env variables like it was. Then we can reuse it in the plugins, but I guess all the plugins have .travis.yml has to convert to GitHub actions as well? Or we can leave Travis for plugins for now. Only Core on Github Action. what would be the best approach?

@tsteur commented on November 12th 2021 Member

. Then we can reuse it in the plugins, but I guess all the plugins have .travis.yml has to convert to GitHub actions as well?

That will be needed indeed.

We'll also need to set up the GitHub secrets for each plugin as currently the secrets are configured in Travis.

We'll also need to adjust various docs like https://developer.matomo.org/guides/tests-travis and https://developer.matomo.org/guides/tests-travis-extended

Or we can leave Travis for plugins for now. Only Core on Github Action. what would be the best approach?

I would say they should all use the same system. That will simplify a lot of things.

@github-actions[bot] commented on November 26th 2021 Contributor

If you don't want this PR to be closed automatically in 28 days then you need to assign the label 'Do not close'.

Powered by GitHub Issue Mirror