Skip to content

Conversation

@nickvergessen
Copy link
Member

@nickvergessen nickvergessen commented Aug 27, 2024

The CI runs of this app are too heavy.
Please reduce the matrix to something suitable. The change in name: unit tests and linting looks quite "decent".
The name: API tests change was just to prevent this PR from swallowing another 1h55m of CI time.

A recommended matrix is:

  • Test each PHP version (8.1, 8.2, 8.3) with 1 database (mysql) against 1 server (stable30)
  • Test each additional database (postgres) against 1 PHP (8.1 installs fastest) the latest supported server (stable30)
  • Test each older server (stable26, …, stable29) with 1 PHP version (8.1 installs fastest) against 1 database (mysql) only

Matrix

Job Before After
unit tests and linting 17 7
X stable30 php8.1
stable30 php8.2
stable30 php8.3
stable29 php8.0
stable29 php8.1
stable29 php8.2
stable29 php8.3
stable28 php8.0
stable28 php8.1
stable28 php8.2
stable28 php8.3
stable27 php8.0
stable27 php8.1
stable27 php8.2
stable26 php8.0
stable26 php8.1
stable26 php8.2
stable30 php8.1
stable30 php8.2
stable30 php8.3
stable29 php8.1
stable28 php8.1
stable27 php8.0
stable26 php8.0
API tests 34 8
X stable30 php8.1 mysql
stable30 php8.2 mysql
stable30 php8.3 mysql
stable30 php8.1 pgsql
stable30 php8.2 pgsql
stable30 php8.3 pgsql
stable29 php8.0 mysql
stable29 php8.1 mysql
stable29 php8.2 mysql
stable29 php8.3 mysql
stable29 php8.0 pgsql
stable29 php8.1 pgsql
stable29 php8.2 pgsql
stable29 php8.3 pgsql
stable28 php8.0 mysql
stable28 php8.1 mysql
stable28 php8.2 mysql
stable28 php8.3 mysql
stable28 php8.0 pgsql
stable28 php8.1 pgsql
stable28 php8.2 pgsql
stable28 php8.3 pgsql
stable27 php8.0 mysql
stable27 php8.1 mysql
stable27 php8.2 mysql
stable27 php8.0 pgsql
stable27 php8.1 pgsql
stable27 php8.2 pgsql
stable26 php8.0 mysql
stable26 php8.1 mysql
stable26 php8.2 mysql
stable26 php8.0 pgsql
stable26 php8.1 pgsql
stable26 php8.2 pgsql
stable30 php8.1 mysql
stable30 php8.2 mysql
stable30 php8.3 mysql
stable30 php8.1 pgsql
stable29 php8.1 mysql
stable28 php8.1 mysql
stable27 php8.0 mysql
stable26 php8.0 mysql

@come-nc
Copy link

come-nc commented Aug 27, 2024

icewind1991/nextcloud-version-matrix may help here, no?
Like in https://github.com/nextcloud/.github/blob/master/workflow-templates/phpunit-pgsql.yml for instance.

SagarGi
SagarGi previously approved these changes Aug 28, 2024
Copy link
Contributor

@SagarGi SagarGi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This app needs to support available enterprise version of Nextcloud. we are soon excluding stable26. But if this app is only one concerned for having more CI times then we can make it less may be running database to only mysql. We are keeping php 8.0 because we can install Nextcloud-26 and Nextcloud-27. @nickvergessen

@SagarGi SagarGi dismissed their stale review August 28, 2024 03:41

mistakenly approved

Copy link
Contributor

@SagarGi SagarGi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@individual-it what do you think?

@SagarGi SagarGi requested a review from individual-it August 28, 2024 04:09
Signed-off-by: Joas Schilling <[email protected]>
@nickvergessen nickvergessen force-pushed the ci/noid/reduce-matrix branch from a47f9de to 4ac9daf Compare August 28, 2024 05:19
@individual-it
Copy link
Collaborator

If CI time is a concern, we can merge this reduced matrix and run the whole set of tests pre-release. That would introduce a little bit more time and risk for the release process, but that should be fine

@SagarGi
Copy link
Contributor

SagarGi commented Aug 29, 2024

If CI time is a concern, we can merge this reduced matrix and run the whole set of tests pre-release. That would introduce a little bit more time and risk for the release process, but that should be fine

@individual-it for PR lets keep it this way. But lets run all of the things (all tests with different versions) in nightly which would be better for release process also!

@nickvergessen
Copy link
Member Author

But lets run all of the things (all tests with different versions) in nightly which would be better for release process also!

The posted matrix (testing each Nextcloud version at least once, testing each Database version at least once, testing each PHP version at least once) should really be enough.
The likeliness of something failing only on a specific PHP+DB+Server combination is so low, that it does not qualify the burned energy here:
https://github.com/nextcloud/integration_openproject/actions/runs/10605760605
Every night you burn more than 8h of CI server time.
You can really be a bit more conservative here and think a little greener 🍃
Not even the Nextcloud server is tested in all combinations.

If you ever encountered an issue that qualifies such a huge matrix I'm happy to learn about the combination of PHP+DB+Server that failed while all other PHP versions passed on the same DB+Server, all other DBs passed on the same PHP+Server and all other Server passed on the same PHP+DB.

@SagarGi
Copy link
Contributor

SagarGi commented Aug 29, 2024

But lets run all of the things (all tests with different versions) in nightly which would be better for release process also!

The posted matrix (testing each Nextcloud version at least once, testing each Database version at least once, testing each PHP version at least once) should really be enough. The likeliness of something failing only on a specific PHP+DB+Server combination is so low, that it does not qualify the burned energy here: https://github.com/nextcloud/integration_openproject/actions/runs/10605760605 Every night you burn more than 8h of CI server time. You can really be a bit more conservative here and think a little greener 🍃 Not even the Nextcloud server is tested in all combinations.

If you ever encountered an issue that qualifies such a huge matrix I'm happy to learn about the combination of PHP+DB+Server that failed while all other PHP versions passed on the same DB+Server, all other DBs passed on the same PHP+Server and all other Server passed on the same PHP+DB.

@nickvergessen sure may be lets do it your way if as it seems to be consuming more CI server time. I will once check how this PR goes for a scheduled event and then merge it. Thanks!

Copy link
Contributor

@SagarGi SagarGi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 👍 Thanks @nickvergessen for taking care of it!

@github-actions
Copy link

JS Code Coverage

Coverage after merging ci/noid/reduce-matrix into master will be
87.72%
Coverage Report
FileStmtsBranchesFuncsLinesUncovered Lines
src
   adminSettings.js0%0%0%0%1, 1, 10–19, 2, 20–25, 3–9
   bootstrap.js0%0%0%0%1, 1–7
   dashboard.js0%0%0%0%1, 1, 10–19, 2, 20–25, 3–9
   fileActions.js0%0%0%0%1, 1, 10–17, 2–9
   personalSettings.js0%0%0%0%1, 1, 10–19, 2, 20–25, 3–9
   projectTab.js0%0%0%0%1, 1, 10–19, 2, 20–29, 3, 30–39, 4, 40–49, 5, 50–59, 6, 60–66, 7–9
   reference.js0%0%0%0%1, 1, 10–19, 2, 20–29, 3, 30–39, 4, 40–49, 5, 50–59, 6, 60, 7–9
   utils.js71.43%33.33%50%73.85%10–14, 17–26, 6–9
src/components
   AdminSettings.vue96.16%93.83%95.16%96.52%1033–1036, 1072–1074, 1094–1097, 518–519, 655, 667–669, 681, 681, 681–686, 689–690, 694–695, 698–699, 703–704, 714–719, 779–781, 793–796, 809–811, 996–998
   OAuthConnectButton.vue91.54%75%100%92.98%59–64, 67–71
   PersonalSettings.vue90.16%94.44%85.71%89.88%100, 110–115, 118–127, 99
src/components/admin
   FieldValue.vue100%100%100%100%
   FormHeading.vue100%100%100%100%
   ProjectFolderError.vue100%100%100%100%
   TermsOfServiceUnsigned.vue100%100%100%100%
   TextInput.vue100%100%100%100%
src/components/icons
   ClippyIcon.vue100%100%100%100%
   OpenProjectIcon.vue100%100%100%100%
src/components/settings
   CheckBox.vue100%100%100%100%
   SettingsTitle.vue96.74%85.71%100%97.53%46–48
src/components/tab
   EmptyContent.vue97.22%89.47%100%98.06%92–96
   SearchInput.vue95.27%92.96%94.74%95.73%134–135, 188, 199–204, 263–265, 281–283, 287–292
   WorkPackage.vue86.10%73.17%93.33%87.46%102–111, 124–126, 137–141, 151–153, 171–177, 215, 215–220, 220, 220–231, 76–77
src/filesPlugin
   filesPlugin.js0%0%0%0%1, 1, 10, 100–104, 11–19, 2, 20–29, 3, 30–39, 4, 40–49, 5, 50–59, 6, 60–69, 7, 70–79, 8, 80–89, 9, 90–99
   filesPluginLessThan28.js0%0%0%0%1, 1, 10–19, 2, 20–29, 3, 30–39, 4, 40–49, 5, 50–59, 6, 60–69, 7, 70–78, 8–9
src/utils
   workpackageHelper.js89.72%82.14%88.89%92.02%153–154, 18–22, 29, 29, 31, 31, 45–47, 49, 49, 49–51, 94–99
src/views
   CreateWorkPackageModal.vue94.34%86.54%91.67%95.50%353–355, 358, 459–462, 467–472, 477–482, 488–491, 494, 510, 510, 550–554, 564–566, 585–587, 617–619, 641–643, 652–656
   Dashboard.vue77.40%80.39%61.90%78.01%103–108, 115, 119–120, 125, 128, 131–134, 139–141, 182–188, 194–197, 199–209, 238–246, 259–273, 51, 63, 88–91, 98–99
   LinkMultipleFilesModal.vue100%100%100%100%
   ProjectsTab.vue94.91%94.23%93.33%95.07%113–116, 142, 153–154, 188–198, 246–248

@github-actions
Copy link

PHP Code Coverage

Coverage after merging ci/noid/reduce-matrix into master will be
60.22%
Coverage Report
FileStmtsBranchesFuncsLinesUncovered Lines
server/apps/integration_openproject/lib
   Capabilities.php0%100%0%0%19, 26–35
server/apps/integration_openproject/lib/AppInfo
   Application.php7.41%100%25%6%101, 103–106, 108, 110, 112, 116–119, 121–132, 134, 137, 141, 145–147, 74–76, 79–83, 85–90, 92–93, 96
server/apps/integration_openproject/lib/BackgroundJob
   RemoveExpiredDirectUploadTokens.php0%100%0%0%42, 44–46, 55–56
server/apps/integration_openproject/lib/Controller
   ConfigController.php64.76%100%50%65.37%135, 152–153, 155, 157–159, 161–164, 167–168, 170, 212, 222, 226–228, 292–296, 406–408, 410–412, 417–420, 461, 549–552, 554–555, 558, 566–570, 581, 595–598, 606–607, 611–614, 628–632, 634–635, 637–653, 670–675, 677–678, 680–682, 685, 687–703, 716–723, 725–728, 730–734, 743–748
   DirectDownloadController.php0%100%0%0%36–38, 53–55, 57–64
   DirectUploadController.php71.03%100%100%70.21%141–143, 187–189, 200, 204–207, 209, 219, 226, 242–244, 246–247, 250–255, 258, 260, 268–270, 276–278, 286–288, 303–305, 324, 329, 335
   FilesController.php72.95%100%83.33%72.41%181–182, 244, 249–252, 254, 256–269, 272–273, 275–276, 280–283, 286, 292
   OpenProjectAPIController.php85.60%100%80%85.92%139, 180, 204, 230–233, 236–243, 245–249, 251, 263, 272, 290, 299, 369, 371, 421, 423, 443, 445, 492, 494, 520–523, 526–530, 532, 737–741, 96
server/apps/integration_openproject/lib/Dashboard
   OpenProjectWidget.php0%100%0%0%101, 108, 115–116, 118, 120–137, 69–73, 80, 87, 94
server/apps/integration_openproject/lib/Exception
   OpenprojectAvatarErrorException.php100%100%100%100%
   OpenprojectErrorException.php100%100%100%100%
   OpenprojectFileNotUploadedException.php100%100%100%100%
   OpenprojectGroupfolderSetupConflictException.php100%100%100%100%
   OpenprojectResponseException.php100%100%100%100%
   OpenprojectUnauthorizedUserException.php0%100%0%0%16
server/apps/integration_openproject/lib/Listener
   BeforeGroupDeletedListener.php0%100%0%0%48, 56–57, 60–63, 65
   BeforeNodeInsideOpenProjectGroupfilderChangedListener.php0%100%0%0%41–43, 47–50, 52, 54, 57–58, 60, 62–65, 67–70, 72–74
   BeforeUserDeletedListener.php0%100%0%0%48, 55–56, 58–61, 63
   LoadAdditionalScriptsListener.php0%100%0%0%17, 20–21, 24–26, 28
   LoadSidebarScript.php0%100%0%0%100, 102, 104–105, 107, 109, 111, 113–122, 75–91, 96–97, 99
   OpenProjectReferenceListener.php0%100%0%0%53–54, 57–58, 61–62, 64–67
   TermsOfServiceEventListener.php0%100%0%0%59–60, 65–66, 68–69, 71–73, 76–80
   UserChangedListener.php0%100%0%0%52, 59–60, 63–68, 71
server/apps/integration_openproject/lib/Migration
   Version2001Date20221213083550.php0%100%0%0%47, 57–65, 67–75, 77–79, 81
   Version2310Date20230116153411.php0%100%0%0%46, 49–52, 54–79, 81–82, 84
   Version2400Date20230504144300.php0%100%0%0%47, 57–60
   Version2640Date20240628114301.php0%100%0%0%52, 64–66, 69–70, 73
server/apps/integration_openproject/lib/Reference
   WorkPackageReferenceProvider.php51.67%100%25%58.33%102, 108–111, 114–116, 119, 123, 157, 165–166, 174, 52, 59, 66, 73–75
server/apps/integration_openproject/lib/Search
   OpenProjectSearchProvider.php0%100%0%0%103–104, 107–118, 121–122, 124–125, 128–137, 139–143, 66–69, 76, 83, 91, 93, 96
   OpenProjectSearchResultEntry.php100%100%100%100%
server/apps/integration_openproject/lib/Service
   DatabaseService.php42.31%100%60%40.43%100–102, 125–129, 131, 80–93, 95–99
   DirectDownloadService.php88.46%100%100%87.50%65–66, 68
   DirectUploadService.php42.86%100%66.67%40%112, 118, 79–82, 84–92
   OauthService.php0%100%0%0%109–116, 45–47, 56–66, 68, 70–72, 74–80, 89–98
   OpenProjectAPIService.php75.66%100%75.93%75.64%1036–1038, 1040–1042, 1045–1049, 1051–1053, 1062–1065, 1068–1070, 1072, 1075–1080, 1084–1085, 1116–1119, 1138–1145, 1154–1155, 1162–1164, 1166–1169, 1173, 1182, 1200, 1202–1205, 1207–1212, 1354, 1366, 1369, 1391, 1394, 1404–1409, 1534–1536, 1538–1539, 1543–1544, 1546, 1548, 180–184,

@nickvergessen nickvergessen merged commit b3774dd into master Aug 29, 2024
@skjnldsv skjnldsv deleted the ci/noid/reduce-matrix branch August 29, 2024 09:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants