@peterhashair opened this Pull Request on January 23rd 2022 Contributor

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

@peterhashair commented on February 8th 2022 Contributor

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

@peterhashair commented on February 10th 2022 Contributor

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?

  • [ ] Doc] Documentation for development guild

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

@github-actions[bot] commented on February 21st 2022 Contributor

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

@peterhashair commented on March 6th 2022 Contributor

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

@tsteur commented on March 7th 2022 Member

@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:

image
@github-actions[bot] commented on March 17th 2022 Contributor

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

@github-actions[bot] commented on March 24th 2022 Contributor

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

@github-actions[bot] commented on April 1st 2022 Contributor

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

@github-actions[bot] commented on April 9th 2022 Contributor

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

@github-actions[bot] commented on April 20th 2022 Contributor

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

@peterhashair commented on May 6th 2022 Contributor

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

@github-actions[bot] commented on June 10th 2022 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 July 6th 2022 Contributor

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

@justinvelluppillai commented on August 4th 2022 Member

@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

@peterhashair commented on August 9th 2022 Contributor

@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 commented on August 9th 2022 Member

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

@peterhashair commented on August 10th 2022 Contributor

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

@peterhashair commented on August 15th 2022 Contributor

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.

@peterhashair commented on August 16th 2022 Contributor

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

@peterhashair commented on August 23rd 2022 Contributor

@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

@peterhashair commented on September 8th 2022 Contributor

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

@sgiehl commented on September 8th 2022 Member

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'
@peterhashair commented on September 20th 2022 Contributor

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

@peterhashair commented on September 28th 2022 Contributor

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.

@justinvelluppillai commented on September 28th 2022 Member

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

@peterhashair commented on September 28th 2022 Contributor

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

@justinvelluppillai commented on September 28th 2022 Member

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

@bx80 commented on September 28th 2022 Contributor

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

@peterhashair commented on September 28th 2022 Contributor

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

@github-actions[bot] commented on October 6th 2022 Contributor

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

Powered by GitHub Issue Mirror