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.

Sign up for GitHub

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

Jump to bottom

[nomerge] Ocfs merge stray shares uid fixes #1068

Closed
wants to merge 15 commits into from
Closed

[nomerge] Ocfs merge stray shares uid fixes #1068

wants to merge 15 commits into from

Conversation

PVince81
Copy link
Contributor

No description provided.

butonic and others added 14 commits July 30, 2020 14:46
@butonic
ocfs: lookup user to render template properly
8bbbd78 
Signed-off-by: Jörn Friedrich Dreyer <jfd@butonic.de>
@butonic
refactor, configurable addr
d4a46c5 
Signed-off-by: Jörn Friedrich Dreyer <jfd@butonic.de>
@butonic
use correct userprovider service
e8b185b 
Signed-off-by: Jörn Friedrich Dreyer <jfd@butonic.de>
OCFS: Change default user layout
ccc3220
OCFS update configs for user provider svc
4b0bff3 
The user provider service now needs to be specified in the
configurations
@refs
ignore stray shares
50420e1
@refs
Merge branch 'master' into fix-error-propagation
95927bf
Fix Litmus test to use UUID in DAV URL
82f1bd3 
Fixed Drone config to use hard-coded UUID for einstein in the DAV URL.
Adjust PR number in changelog
0acc7d9
@refs
stray block
7650a6d
@refs
log reason of STAT failure
18ff637
@refs
fix warnings and offenses
c433b7d
@refs
add changelog
6bd190f
Merge branch 'ocfs-lookup-userid-update' into temp-merge-madness
1ae0006
@PVince81 PVince81 changed the title Ocfs merge stray shares uid fixes [nomerge] Ocfs merge stray shares uid fixes Aug 10, 2020
@PVince81
Copy link
Contributor Author

just for CI, it's #1052 and #1064

@PVince81 PVince81 mentioned this pull request Aug 10, 2020
Merged
@refs
Copy link
Member

refs commented Aug 10, 2020 •
edited
Loading

transcript from CI:

--- Failed scenarios:

/drone/src/tmp/testrunner/tests/acceptance/features/apiShareManagementBasic/createShare.feature:380
/drone/src/tmp/testrunner/tests/acceptance/features/apiShareManagementBasic/createShare.feature:381
/drone/src/tmp/testrunner/tests/acceptance/features/apiSharePublicLink1/changingPublicLinkShare.feature:74
/drone/src/tmp/testrunner/tests/acceptance/features/apiSharePublicLink1/changingPublicLinkShare.feature:117
/drone/src/tmp/testrunner/tests/acceptance/features/apiSharePublicLink1/changingPublicLinkShare.feature:139
/drone/src/tmp/testrunner/tests/acceptance/features/apiSharePublicLink1/changingPublicLinkShare.feature:162
/drone/src/tmp/testrunner/tests/acceptance/features/apiSharePublicLink1/changingPublicLinkShare.feature:209
/drone/src/tmp/testrunner/tests/acceptance/features/apiSharePublicLink1/changingPublicLinkShare.feature:232
/drone/src/tmp/testrunner/tests/acceptance/features/apiSharePublicLink1/changingPublicLinkShare.feature:255
/drone/src/tmp/testrunner/tests/acceptance/features/apiVersions/fileVersions.feature:406
/drone/src/tmp/testrunner/tests/acceptance/features/apiVersions/fileVersions.feature:407
/drone/src/tmp/testrunner/tests/acceptance/features/apiVersions/fileVersions.feature:459
/drone/src/tmp/testrunner/tests/acceptance/features/apiVersions/fileVersions.feature:460
/drone/src/tmp/testrunner/tests/acceptance/features/apiWebdavMove1/moveFolder.feature:98
/drone/src/tmp/testrunner/tests/acceptance/features/apiWebdavMove1/moveFolder.feature:99
/drone/src/tmp/testrunner/tests/acceptance/features/apiWebdavMove1/moveFolder.feature:122
/drone/src/tmp/testrunner/tests/acceptance/features/apiWebdavMove1/moveFolder.feature:123
/drone/src/tmp/testrunner/tests/acceptance/features/apiWebdavMove1/moveFolder.feature:124
/drone/src/tmp/testrunner/tests/acceptance/features/apiWebdavMove1/moveFolder.feature:125
/drone/src/tmp/testrunner/tests/acceptance/features/apiWebdavMove1/moveFolder.feature:126
/drone/src/tmp/testrunner/tests/acceptance/features/apiWebdavMove1/moveFolder.feature:127
/drone/src/tmp/testrunner/tests/acceptance/features/apiWebdavMove1/moveFolder.feature:128
/drone/src/tmp/testrunner/tests/acceptance/features/apiWebdavMove1/moveFolder.feature:129
/drone/src/tmp/testrunner/tests/acceptance/features/apiWebdavMove1/moveFolder.feature:130
/drone/src/tmp/testrunner/tests/acceptance/features/apiWebdavMove1/moveFolder.feature:131
/drone/src/tmp/testrunner/tests/acceptance/features/apiWebdavMove1/moveFolderToBlacklistedName.feature:34
/drone/src/tmp/testrunner/tests/acceptance/features/apiWebdavMove1/moveFolderToBlacklistedName.feature:35
/drone/src/tmp/testrunner/tests/acceptance/features/apiWebdavMove2/moveFile.feature:21
/drone/src/tmp/testrunner/tests/acceptance/features/apiWebdavMove2/moveFile.feature:22
/drone/src/tmp/testrunner/tests/acceptance/features/apiWebdavMove2/moveFile.feature:34
/drone/src/tmp/testrunner/tests/acceptance/features/apiWebdavMove2/moveFile.feature:35
/drone/src/tmp/testrunner/tests/acceptance/features/apiWebdavMove2/moveFile.feature:45
/drone/src/tmp/testrunner/tests/acceptance/features/apiWebdavMove2/moveFile.feature:46
/drone/src/tmp/testrunner/tests/acceptance/features/apiWebdavMove2/moveFile.feature:57
/drone/src/tmp/testrunner/tests/acceptance/features/apiWebdavMove2/moveFile.feature:58
/drone/src/tmp/testrunner/tests/acceptance/features/apiWebdavMove2/moveFile.feature:68
/drone/src/tmp/testrunner/tests/acceptance/features/apiWebdavMove2/moveFile.feature:69
/drone/src/tmp/testrunner/tests/acceptance/features/apiWebdavMove2/moveFile.feature:231
/drone/src/tmp/testrunner/tests/acceptance/features/apiWebdavMove2/moveFile.feature:232
/drone/src/tmp/testrunner/tests/acceptance/features/apiWebdavMove2/moveFile.feature:243
/drone/src/tmp/testrunner/tests/acceptance/features/apiWebdavMove2/moveFile.feature:244
/drone/src/tmp/testrunner/tests/acceptance/features/apiWebdavMove2/moveFile.feature:296
/drone/src/tmp/testrunner/tests/acceptance/features/apiWebdavMove2/moveFile.feature:297
/drone/src/tmp/testrunner/tests/acceptance/features/apiWebdavMove2/moveFile.feature:305
/drone/src/tmp/testrunner/tests/acceptance/features/apiWebdavMove2/moveFile.feature:306
/drone/src/tmp/testrunner/tests/acceptance/features/apiWebdavMove2/moveFile.feature:326
/drone/src/tmp/testrunner/tests/acceptance/features/apiWebdavMove2/moveFile.feature:337
/drone/src/tmp/testrunner/tests/acceptance/features/apiWebdavMove2/moveFile.feature:338
/drone/src/tmp/testrunner/tests/acceptance/features/apiWebdavMove2/moveFile.feature:339
/drone/src/tmp/testrunner/tests/acceptance/features/apiWebdavMove2/moveFile.feature:340
/drone/src/tmp/testrunner/tests/acceptance/features/apiWebdavMove2/moveFile.feature:341
/drone/src/tmp/testrunner/tests/acceptance/features/apiWebdavMove2/moveFile.feature:342
/drone/src/tmp/testrunner/tests/acceptance/features/apiWebdavMove2/moveFile.feature:343
/drone/src/tmp/testrunner/tests/acceptance/features/apiWebdavMove2/moveFile.feature:344
/drone/src/tmp/testrunner/tests/acceptance/features/apiWebdavMove2/moveFile.feature:345
/drone/src/tmp/testrunner/tests/acceptance/features/apiWebdavMove2/moveFile.feature:346
/drone/src/tmp/testrunner/tests/acceptance/features/apiWebdavMove2/moveFile.feature:347
/drone/src/tmp/testrunner/tests/acceptance/features/apiWebdavMove2/moveFile.feature:348
/drone/src/tmp/testrunner/tests/acceptance/features/apiWebdavMove2/moveFileToBlacklistedName.feature:30
/drone/src/tmp/testrunner/tests/acceptance/features/apiWebdavMove2/moveFileToBlacklistedName.feature:31
/drone/src/tmp/testrunner/tests/acceptance/features/apiWebdavProperties1/setFileProperties.feature:57
/drone/src/tmp/testrunner/tests/acceptance/features/apiWebdavProperties1/setFileProperties.feature:58

475 scenarios (413 passed, 62 failed)
3705 steps (3530 passed, 62 failed, 113 skipped)
7m1.59s (20.32Mb)

@PVince81
Copy link
Contributor Author

PVince81 commented Aug 10, 2020 •
edited
Loading

I have tests/acceptance/features/apiShareManagementBasic/createShare.feature:380 passing locally the first then, then failing with a "405" error, probably due to stray shares.

% TEST_SERVER_URL='http://localhost:20080' \
OCIS_REVA_DATA_ROOT='/drone/srv/tmp/reva' \
SKELETON_DIR="$HOME/work/workspace/owncloud/apps-external/testing/data/apiSkeleton" \
TEST_EXTERNAL_USER_BACKENDS='true' \
REVA_LDAP_HOSTNAME='ldap' \
TEST_OCIS='true' \
TEST_REVA='true' \
BEHAT_FEATURE=tests/acceptance/features/apiShareManagementBasic/createShare.feature:380 \
BEHAT_FILTER_TAGS='~@skipOnOcis&&~@skipOnOcis-OC-Storage' \
make test-acceptance-api

result:

And user "Alice" has created folder "/folder1"

        HTTP status code 405 is not one of the expected values
        Failed asserting that an array contains 405.

@PVince81
Copy link
Contributor Author

PVince81 commented Aug 10, 2020 •
edited
Loading

never mind, it was a local setup issue...

when I run createShare:380 alone with a clean env, the test passes, unlike on CI.

if I run the whole of createShare feature file, then:

OCS status code is not the expected value 100 got 996
        Failed asserting that two strings are equal.
        --- Expected
        +++ Actual
        @@ @@
        -'100'
        +'996'

    /srv/www/htdocs/owncloud/tests/acceptance/features/apiShareManagementBasic/createShare.feature:380
    /srv/www/htdocs/owncloud/tests/acceptance/features/apiShareManagementBasic/createShare.feature:381

@PVince81
Copy link
Contributor Author

I've pushed the move fix, and now only two tests are failing:

tests/acceptance/features/apiShareManagementBasic/createShare.feature:380
tests/acceptance/features/apiShareManagementBasic/createShare.feature:381

sadly these pass for me when running the whole of createShare

@PVince81
Copy link
Contributor Author

detail:

  @skipOnOcV10 @issue-ocis-reva-372 @issue-ocis-reva-243
  Scenario Outline: sharing subfolder of already shared folder, GET result is correct                                      # /drone/src/tmp/testrunner/tests/acceptance/features/apiShareManagementBasic/createShare.feature:352
    Given using OCS API version "<ocs_api_version>"                                                                        # FeatureContext::usingOcsApiVersion()
    And these users have been created with default attributes and without skeleton files:                                  # FeatureContext::theseUsersHaveBeenCreatedWithDefaultAttributesAndWithoutSkeletonFiles()
      | username |
      | Brian    |
      | Carol    |
      | David    |
      | Emily    |
    And user "Alice" has created folder "/folder1"                                                                         # FeatureContext::userHasCreatedFolder()
    And user "Alice" has shared folder "/folder1" with user "Brian"                                                        # FeatureContext::userHasSharedFileWithUserUsingTheSharingApi()
    And user "Alice" has shared folder "/folder1" with user "Carol"                                                        # FeatureContext::userHasSharedFileWithUserUsingTheSharingApi()
    And user "Alice" has created folder "/folder1/folder2"                                                                 # FeatureContext::userHasCreatedFolder()
    And user "Alice" has shared folder "/folder1/folder2" with user "David"                                                # FeatureContext::userHasSharedFileWithUserUsingTheSharingApi()
    And user "Alice" has shared folder "/folder1/folder2" with user "Emily"                                                # FeatureContext::userHasSharedFileWithUserUsingTheSharingApi()
    When user "Alice" sends HTTP method "GET" to OCS API endpoint "/apps/files_sharing/api/v1/shares"                      # OCSContext::userSendsToOcsApiEndpoint()
    Then the OCS status code should be "<ocs_status_code>"                                                                 # OCSContext::theOCSStatusCodeShouldBe()
    And the HTTP status code should be "<http_status_code>"                                                                # FeatureContext::thenTheHTTPStatusCodeShouldBe()
    And user "Alice" sends HTTP method "GET" to OCS API endpoint "/apps/files_sharing/api/v1/shares?path=/folder1/folder2" # OCSContext::userSendsToOcsApiEndpoint()
    And the response should contain 2 entries                                                                              # FeatureContext::checkingTheResponseEntriesCount()
    And folder "/folder1" should not be included as path in the response                                                   # FeatureContext::checkSharedFileAsPathNotInResponse()
    And folder "/folder2" should be included as path in the response                                                       # FeatureContext::checkSharedFileAsPathInResponse()

    Examples:
      | ocs_api_version | http_status_code | ocs_status_code |
      | 1               | 200              | 996             |
        OCS status code is not the expected value 996 got 100
        Failed asserting that two strings are equal.
        --- Expected
        +++ Actual
        @@ @@
        -'996'
        +'100'
      | 2               | 500              | 996             |
        OCS status code is not the expected value 996 got 200
        Failed asserting that two strings are equal.
        --- Expected
        +++ Actual
        @@ @@
        -'996'
        +'200'

@PVince81
Copy link
Contributor Author

linked issues: https://github.com/owncloud/ocis-reva/issues/372 and https://github.com/owncloud/ocis-reva/issues/243

but doesn't explain why these tests pass locally but not on CI

@butonic
Copy link
Contributor

butonic commented Aug 10, 2020 •
edited
Loading

so the tests currently expect the list shares request to fail, but we actually implemented that endpoint and it should work. Or in the words of our testsuite: https://github.com/owncloud/core/pull/37754/files#diff-129a7916711570e3ddb2eb9f655ce577

The question remains: why is the test failing locally (which means the list shares endpoint actually returns an error).

it might have to do with the share manager not being started? or restarted? or reset properly?

@PVince81
Copy link
Contributor Author

PVince81 commented Aug 10, 2020 •
edited
Loading

I've now tried running my local createShare.feature with core's 9ebf886779b98fc3f78e11cf138246f80d5cab39 which is the commit on which Drone is running and I also consistently receive the failure, so local and drone are on the same page now.

When I had the branch testFixOCISPR409 checked out the test passed.

@PVince81
Copy link
Contributor Author

one of the PRs got merged.

closing in favor of #1064 where we'll test with testFixOCISPR409

@PVince81 PVince81 closed this Aug 10, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@glpatcern glpatcern Awaiting requested review from glpatcern glpatcern will be requested when the pull request is marked ready for review glpatcern is a code owner

@diocas diocas Awaiting requested review from diocas diocas will be requested when the pull request is marked ready for review diocas is a code owner

@jessegeens jessegeens Awaiting requested review from jessegeens jessegeens will be requested when the pull request is marked ready for review jessegeens is a code owner

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@PVince81 @refs @butonic