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

Description:

All the files that failed will be downloadable in the artifact at the moment. Add a new command for GitHub action. Not sure if that needed ./console tests:sync-ui-screenshots -a github -r matomo-org/matomo 1528756403 just used for my local screen compare and update image.

Failed UI

image

Group all the bash to .github/scripts/bash/prepare.sh
This file split PHP XML into smaller fragments tests/PHPUnit/formatXML.php

MYSQLI adapter setup

mysqli.allow_local_infile = On

Submodule implementation:

To set up Submodule on GitHub need to update plugin_tests.yml. Once approved, github-action should be 4.x-dev

    env:
      TARGET_BRANCH: github-aciton
      PLUGIN: AnonymousPiwikUsageMeasurement

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'.

@peterhashair commented on November 30th 2021 Contributor

@tsteur convert it to bash, still fixing some ui failed mismatch, should I start covert some of the plugin travis.

@tsteur commented on November 30th 2021 Member

@peterhashair yeah it be great to test a plugin and checking that it works running tests on github actions just by putting in a minor workflow.yml. Not sure if we need to merge this one before to make this work?

Before doing this, maybe someone should also review this and we'd want tests to pass already here ideally?

@peterhashair commented on November 30th 2021 Contributor

@tsteur yes I will fix all the failed tests, I had a preview most of them are tiny fonts different.

@peterhashair commented on December 7th 2021 Contributor

@tsteur convert coupe plugins, it seems working, probably needs someone to review.

@sgiehl commented on December 7th 2021 Member

@peterhashair As I just updated my tests for device detector: The php install action meanwhile supports PHP 8.1. So in theory you could change that here, so it will already run tests on PHP 8.1

@peterhashair commented on December 7th 2021 Contributor

@sgiehl 8.1 works. do you want to update all the tests to 8.1?

@sgiehl commented on December 7th 2021 Member

@peterhashair On travis we had builds running on PHP7.2 and PHP8, to cover minimum and maximum supported version. There I would have updated the PHP8 builds only. I didn't yet have a detailed look at this PR, so not sure which builds are running on which PHP version...

@peterhashair commented on December 8th 2021 Contributor

@sgiehl do you want to start reviewing this, it's kind of working, and all the submodules are passing the test except one screenshot I haven't fixed. Also got an update for ui-test-view https://github.com/matomo-org/ui-tests-viewer/pull/37/files.

@sgiehl commented on December 13th 2021 Member

@peterhashair Sure I can start reviewing that one. Do you want me to have a look at something specific or is this one fully finished? If so please ensure you had a look yourself, that only required changes are included and that our new test suites are testing the same as on travis (e.g. same suites on same PHP versions). I don't want to waste time looking at unfinished PRs.

@peterhashair commented on December 13th 2021 Contributor

@sgiehl Did self-review and Update the PHP version Should be the same as the Travis now. Still needs to clean up the Travis folder and relates.
There is a test failure because 7.2.34-28+ubuntu20.04.1+deb.sury.org+1 still needs to be fixed. currently working on it.
All the screenshot updates are fonts-related.

But I guess this didn't need reviewed before this https://github.com/matomo-org/ui-tests-viewer/pull/37/files deployed first otherwise, the upload won't work.

PHP version listed.

  • All the Integration, unit and system and core tests run on 7.2 and 8.0,
  • javascript test run on 7.2
  • UI test run on 7.2.
@peterhashair commented on December 20th 2021 Contributor

I guess we missing this part in here
./console development:sync-system-test-processed {buildnumber}.

@sgiehl commented on December 21st 2021 Member

@peterhashair the system test builds used to upload their artifacts to our build server as well.

@peterhashair commented on December 30th 2021 Contributor

@sgiehl modified this a little bit, the Mariadb version using the in github action is higher than 10.1.1. DB::isOptimizeInnoDBSupported() is returning true. added the bottom function for the test.

      if (DB::isOptimizeInnoDBSupported()) {
            $this->assertTrue(Db::optimizeTables(array('table3', 'table4')) !== false);
        } else {
            $this->assertTrue(Db::optimizeTables(array('table3', 'table4')) === false);
        }
@sgiehl commented on February 2nd 2022 Member

@peterhashair Is #18679 a replacement for this one

@peterhashair commented on February 2nd 2022 Contributor

@sgiehl yes, this one got some extra changes on screenshot, which doesn't needed

This Pull Request was closed on February 13th 2022
Powered by GitHub Issue Mirror