Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

tests: check folder exist before creation #697

Merged
merged 3 commits into from
Sep 11, 2024

Conversation

SagarGi
Copy link
Collaborator

@SagarGi SagarGi commented Sep 6, 2024

Description

This PR added code to the test to determine whether the file or folder existed before the folder's creation.

Problem

https://github.com/nextcloud/integration_openproject/blob/master/tests/acceptance/features/api/setup.feature#L545
Previously, when a folder was created and it already existed, the server would return a 405 status code. But, now it is returning a 500 status code.

https://github.com/nextcloud/integration_openproject/blob/master/tests/acceptance/features/api/setup.feature#L679
Similarly, in case of the PROFIND, it is also returning a 500 status code.

Due to this reasons, there raise CI failure

    And user "OpenProject" has created folder "/OpenProject/project-abc"                                              # FeatureContext::userHasCreatedFolder()
      HTTP status code was not 201 while trying to create folder '/OpenProject' for user 'OpenProject'
      Failed asserting that an array contains 500.

NOTE: This problem raise on image juliushaertl/nextcloud-dev-php83:20231202-1 since on CI this image is used.

Workpackage

Steps to reproduce

  1. Run nextcloud using following command
docker run --rm -p 8080:80 -e SERVER_BRANCH=stable30 -e NEXTCLOUD_AUTOINSTALL=YES -e NEXTCLOUD_AUTOINSTALL_APPS="groupfolders integration_openproject" -v ~/www/stable30/apps:/var/www/html/apps-extra/appid ghcr.io/juliushaertl/nextcloud-dev-php83:20231202-1
  1. Run failure test only
NEXTCLOUD_BASE_URL=http://localhost/stable30 make api-test FEATURE_PATH=tests/acceptance/features/api/setup.feature:491

Screenshots (if appropriate):

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Tests only (no source changes)

Checklist:

  • Code changes
  • Unit tests added
  • Acceptance tests added
  • Updated CHANGELOG.md file

@SagarGi SagarGi force-pushed the check_CI_for_nightly_failure branch from 025d14a to 229bb05 Compare September 6, 2024 04:16
@nabim777 nabim777 force-pushed the check_CI_for_nightly_failure branch 12 times, most recently from a10f8cf to 989b755 Compare September 6, 2024 10:00
@SagarGi SagarGi force-pushed the check_CI_for_nightly_failure branch from d0a2f35 to 10ef318 Compare September 6, 2024 10:25
@nabim777 nabim777 force-pushed the check_CI_for_nightly_failure branch 7 times, most recently from ec7536d to 774db87 Compare September 9, 2024 11:24
@nabim777
Copy link
Collaborator

nabim777 commented Sep 9, 2024

https://github.com/nextcloud/integration_openproject/blob/master/tests/acceptance/features/api/setup.feature#L545
Previously, when a folder was created and it already existed, the server would return a 405 status code. But, now it is returning a 500 status code.

https://github.com/nextcloud/integration_openproject/blob/master/tests/acceptance/features/api/setup.feature#L679
Similarly, in case of the PROFIND, it is also returning a 500 status code.

@individual-it @SagarGi

@individual-it
Copy link
Collaborator

That sounds like a bug in NC, please

  1. have a look if there is already an issue in the server repo and open one if not
  2. Adjust our test code, so that it can react the same regardless if the return is 405 or 500

@SagarGi
Copy link
Collaborator Author

SagarGi commented Sep 10, 2024

That sounds like a bug in NC, please

  1. have a look if there is already an issue in the server repo and open one if not
  2. Adjust our test code, so that it can react the same regardless if the return is 405 or 500

I am quite confused that locally the NC seems to be giving correct status code. But using the docker image that we use in the CI for NC it gives 500. May be there seems to be something wrong with the image. And on the top of that it is flaky. it sometimes pass and sometimes not.

@nabim777 nabim777 self-assigned this Sep 10, 2024
@nabim777 nabim777 changed the title Check for nightly failure tests: check folder exist before creation Sep 10, 2024
Signed-off-by: nabim777 <nabinalemagar019@gmail.com>
@nabim777 nabim777 force-pushed the check_CI_for_nightly_failure branch 5 times, most recently from 9c07cd9 to 6661cc1 Compare September 10, 2024 11:38
@nabim777
Copy link
Collaborator

Issue related to the PROFIND has been fixed in the image juliushaertl/nextcloud-dev-php83:20231202-1
but not in the folder creation

@nabim777 nabim777 marked this pull request as ready for review September 10, 2024 11:42
@nabim777 nabim777 self-requested a review September 10, 2024 11:43
Signed-off-by: nabim777 <nabinalemagar019@gmail.com>
@nabim777 nabim777 force-pushed the check_CI_for_nightly_failure branch from 6661cc1 to 70405fb Compare September 10, 2024 11:52
@nabim777 nabim777 force-pushed the check_CI_for_nightly_failure branch from 8442783 to 1cbd330 Compare September 11, 2024 05:25
Signed-off-by: nabim777 <nabinalemagar019@gmail.com>
@nabim777 nabim777 force-pushed the check_CI_for_nightly_failure branch from 1cbd330 to 79900cf Compare September 11, 2024 06:06
Copy link

JS Code Coverage

Coverage after merging check_CI_for_nightly_failure into release/2.7 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

Copy link

PHP Code Coverage

Coverage after merging check_CI_for_nightly_failure into release/2.7 will be
60.65%
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.php28.30%100%40%27.08%104–112, 123–130, 64–65, 79–87, 89–95
   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, 255, 392–393, <

Copy link
Collaborator Author

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

@nabim777 nabim777 merged commit 5a7cb92 into release/2.7 Sep 11, 2024
15 checks passed
@nabim777 nabim777 deleted the check_CI_for_nightly_failure branch September 11, 2024 06:41
nabim777 added a commit that referenced this pull request Sep 11, 2024
* running the api test only

Signed-off-by: nabim777 <nabinalemagar019@gmail.com>

* checks file and folder exists before creation on tests

Signed-off-by: nabim777 <nabinalemagar019@gmail.com>

* review address

Signed-off-by: nabim777 <nabinalemagar019@gmail.com>

---------

Signed-off-by: nabim777 <nabinalemagar019@gmail.com>
Co-authored-by: nabim777 <nabinalemagar019@gmail.com>
Signed-off-by: nabim777 <nabinalemagar019@gmail.com>
SagarGi added a commit that referenced this pull request Sep 11, 2024
* running the api test only



* checks file and folder exists before creation on tests



* review address



---------

Signed-off-by: nabim777 <nabinalemagar019@gmail.com>
Co-authored-by: Sagar Gurung <46086950+SagarGi@users.noreply.github.com>
Copy link

Hello there,
Thank you so much for taking the time and effort to create a pull request to our Nextcloud project.

We hope that the review process is going smooth and is helpful for you. We want to ensure your pull request is reviewed to your satisfaction. If you have a moment, our community management team would very much appreciate your feedback on your experience with this PR review process.

Your feedback is valuable to us as we continuously strive to improve our community developer experience. Please take a moment to complete our short survey by clicking on the following link: https://cloud.nextcloud.com/apps/forms/s/i9Ago4EQRZ7TWxjfmeEpPkf6

Thank you for contributing to Nextcloud and we hope to hear from you soon!

(If you believe you should not receive this message, you can add yourself to the blocklist.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants