[Github Action] Test 4.x branch #18679

Description:

Fork https://github.com/matomo-org/matomo/pull/18290
update github-action branch has some unnecessary UI updates Test 4.x branch, not sure why but seems like php8.0 MySQLi driver has a poor performance here

UI viewer test:
Because the silex is out of sup. Here is the Symfony version of it, but at the moment only tested the GitHub action part not fully tested Travis. https://github.com/matomo-org/ui-tests-viewer/pull/40

YML, PDF:
when tests failed, the system will upload the artifact as zip and stay on Github for 5 days

Once this is merged to 4.x-dev, we can use the below to wait for the other process finished. Only works on default branch

   workflow_run:
    workflows: [ Build Vue files,PHPCS check ]
    types:
      - completed

Performance

It seems like the MYSQLI is twice slower as the PDO driver, not sure why.

Review

• [ ] Functional review done
• [ ] Potential edge cases thought about (behavior of the code with strange input, with strange internal state or possible interactions with other Matomo subsystems)
• [ ] Usability review done (is anything maybe unclear or think about anything that would cause people to reach out to support)
• [ ] Security review done
• [ ] Wording review done
• [ ] Code review done
• [ ] Tests were added if useful/possible
• [ ] Reviewed for breaking changes
• [ ] Developer changelog updated if needed
• [ ] Documentation added if needed
• [ ] Existing documentation updated if needed

@sgiehl revert some unnecessary code form change. that should make review easier.

advantage:

• The last commit will be canceled the preview build.
• It seems like Github action overall will be 2-10 mins quicker.

disadvantage:

• Entire Pipeline is running as a whole, you can not rerun a single job.
• It seems php8.0 + MySQLI driver is very slow. not sure why could be sth to do with MariaDB.
• Github Action is free on the public repo, but the limit is 20 concurrency jobs per repo. Meaning the max pipeline at the same time are 3 PR requests. Fork will remove the limit if we want to go this path.
• The main test should wait for VUE/PHPCS build finished, then continue. But that option only works on the default branch, which is 4.x-dev. Or we could merge VUE, PHPCS build and test to one pipeline as a step

Push Live Require. Just a list for this to progress.

• [ ] [Review] Code review on this branch.

• [ ] [Decision] do we need Travis and Github action running at the same time, or completely remove Travis
  Suggestion: We keep both running for at least 1 week.

• [ ] [Review] Submodule PR review. Submodule target change to 4. x-dev

• [ ] [Decision] do we allow UI upload for public fork repo. Do we consider displaying the failed UI HTML inside GitHub action?

  • the benefit of fork repo is not limited by concurrency.
    Suggest: we move screen compared to GitHub action, instead of using https://builds-artifacts.matomo.org/

• [ ] Doc] Documentation for development guild

• [ ] [Doc] Documentation for internal staff use.

@tsteur somehow this test on MySQL running really slow, but MariaDB is a little bit quick. Any idea? PHP (SystemTestsCore, 8.0, MYSQLI)

@peterhashair no idea why but you could either try setting testdox=true in the phpunit.xml.dist and then monitor if it's one particular test that takes a while. Or I believe there is also another feature maybe to print how long each test took to execute in phpunit. This would then help you figure out if there's one or few tests that are slow or all of them.

I just checked and testdox gave me this output for example:

@sgiehl move all tests to here https://github.com/matomo-org/github-action-tests makes more sense, but needs a little work, start working on it.

There is one random PHP test that failed remain, JS test won't end.

@peterhashair we would want to use php8.1, if not already then soon. Be worth spending an hour or so understanding what's happening in PHP8.1

@sgiehl yes, should use https://github.com/matomo-org/github-action-tests, just trying to pass all the tests first, then I can covert them to a package.

@sgiehl yes, should use https://github.com/matomo-org/github-action-tests, just trying to pass all the tests first, then I can covert them to a package.

So if you know that your work isn't finished yet, why exactly are you requesting a review?? I'm happy to also have a first look in between to e.g. check if something goes into the right direction. But in that case you need to leave a comment so I'm aware of the current state and what exactly to look at. Otherwise this is a total waste of my time.

@sgiehl sorry about that, just trying to check, if all the test performance is correctly, before converting to package.

Updates, group them into a GitHub package now. Still, some tests fail to try to fix. Seems like GitHub Action provides only run failed jobs options, now all the tests are in one file.

@sgiehl Group them into the package, maybe can give a init review, I believe is just the angular keep failed.

@sgiehl convert the mysql, redis etc to the package, now should be a simple setup. Convert one plugin as a test, here https://github.com/matomo-org/plugin-TasksTimetable/pull/29

@sgiehl fixed the error, but I am confused about that one. Do you mind giving an example?

I don't think, that our tests should in the future only run for pull requests. They should also run for every commit on any base branch (4.x-dev, 5.x-dev) and maybe even manually.

I don't think, that our tests should in the future only run for pull requests. They should also run for every commit on any base branch (4.x-dev, 5.x-dev) and maybe even manually.

We need to make sure that not only pull request have test running, but also our main branches after each commit/merge. Otherwise we can't be sure that tests would still be passing after something is merged.

Guess it would need to be something like

on:
  pull_request:
    types: [opened, synchronize]
  push:
    branches:
      - '**.x-dev'
      - 'next_release'

@justinvelluppillai both Travis and GitHub action is enabled, still, some random PHP failed as expected, also for the UI test, I think we can only use one instead of the other, it seems like the 2 system has some IP or fonts difference, make the UI unique.

The other funny part seems like the php8.1 + MySQLI driver is significantly slower than php7.2 + PDO driver, not too sure why.

As we discussed, we can merge with Travis, temp disables GitHub action UI tests, once we confirm the GitHub action is solid, then we enable the UI tests again.

That sounds good to me @peterhashair as long as we have no security issues and can run them concurrently.

@justinvelluppillai I don't think there is a security issue there, but if someone can review this would be good, still a coupe random fails, but I think that's expected.

@bx80 are you at all familiar with this work to be able to give it a final review?

@justinvelluppillai, @peterhashair I can take a look :+1:

@bx80 all the Gtihub Action codes tests code actually are here: https://github.com/matomo-org/github-action-tests.

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

@peterhashair For the action repo it might be helpful to create a temporary branch and create a PR from main against that branch. It's a lot easier to do a review, if there is a PR that can be commented.

@peterhashair For the action repo it might be helpful to create a temporary branch and create a PR from main against that branch. It's a lot easier to do a review, if there is a PR that can be commented.

refer https://github.com/matomo-org/matomo/pull/19843

@peterhashair For the action repo it might be helpful to create a temporary branch and create a PR from main against that branch. It's a lot easier to do a review, if there is a PR that can be commented.

refer #19843

I guess you misunderstood that. I meant creating a PR on that repo: https://github.com/matomo-org/github-action-tests.
That repo contains the most relevant code and needs a detailed review...

This PR needs to be reviewed together with https://github.com/matomo-org/github-action-tests/pull/2 and one of the plugin PRs like https://github.com/matomo-org/plugin-TasksTimetable/pull/29
Only if all work smoothly together this one can be merged.

@sgiehl do we need anything to get this merged?

As I will be away the next days here a small update on the current state of the github test action.
I have refactored and simplified various parts of the action and added some missing parts that are required for running tests for plugins.
In core, as well as for public and private plugins the action seems to run more or less correctly. There are still a couple of failures that I need to investigate. Once everything works as expected I will start implementing a command to generate the action files for plugins. So we can finally start rolling out the action to all repos.
Will continue to work on it when I'm back.