etags don't propagate correctly sometimes

[SwikritiT]

The following feature failed in recent nightly https://drone.owncloud.com/owncloud/ocis/13031/46/6

Scenario Outline: copying a file inside a publicly shared folder by public changes etag for the sharer # /srv/app/testrunner/tests/acceptance/features/apiWebdavEtagPropagation2/copyFileFolder.feature:121
    Given using <dav_version> DAV path                                                                   # FeatureContext::usingOldOrNewDavPath()
    And user "Alice" has created folder "/upload"                                                        # FeatureContext::userHasCreatedFolder()
    And user "Alice" has uploaded file with content "uploaded content" to "/upload/file.txt"             # FeatureContext::userHasUploadedAFileWithContentTo()
    And user "Alice" has created a public link share with settings                                       # FeatureContext::userHasCreatedAPublicLinkShareWithSettings()
      | path        | upload |
      | permissions | change |
    And user "Alice" has stored etag of element "/"                                                      # WebDavPropertiesContext::userHasStoredEtagOfElement()
    And user "Alice" has stored etag of element "/upload"                                                # WebDavPropertiesContext::userHasStoredEtagOfElement()
    And user "Alice" has stored etag of element "/upload/file.txt"                                       # WebDavPropertiesContext::userHasStoredEtagOfElement()
    And user "Alice" has stored etag of element "/upload/file.txt" on path "/upload/renamedFile.txt"     # WebDavPropertiesContext::userStoresEtagOfElementOnPath()
    When the public copies file "file.txt" to "/renamedFile.txt" using the new public WebDAV API         # PublicWebDavContext::thePublicCopiesFileUsingTheWebDAVApi()
    Then the HTTP status code should be "201"                                                            # FeatureContext::thenTheHTTPStatusCodeShouldBe()
    And these etags should have changed:                                                                 # WebDavPropertiesContext::theseEtagsShouldHaveChanged()
      | user  | path                    |
      | Alice | /                       |
      | Alice | /upload                 |
      | Alice | /upload/renamedFile.txt |
    And these etags should not have changed:                                                             # WebDavPropertiesContext::theseEtagsShouldNotHaveChanged()
      | user  | path             |
      | Alice | /upload/file.txt |

    Examples:
      | dav_version |
      | old         |
      | new         |
      | spaces      |
        WebDavPropertiesContext::theseEtagsShouldHaveChanged
        The etag '"f1767bc7cff8f9573775e70ead429675"' of element '/upload' of user 'Alice' did not change.
        Failed asserting that 1 matches expected 0.

[SwikritiT]

#3988 see this discussion

[kiranparajuli589]

the #3988 discussion concludes:

ETag propagation can always be asynchronous.

so to cope with this, do we introduce some static wait inside the tests before checking for the etag of (grand)parents?
cc @individual-it @phil-davis @SwikritiT

[individual-it]

static wait 😱
just a loop should do, similar to owncloud/core#40180

[phil-davis]

hmmm - I thought that we did something about this some time recently to help make the tests more reliable. But maybe we didn't?

I don't easily see any test code changes related to this. Maybe we really didn't do anything yet, and there is the potential for tests to be a bit flaky.

[kiranparajuli589]

owncloud/core#40180 is merged. With this PR, now the every PROPFIND requests for etag comparisons are checked for HTTP status code 425 and retired 10 times if status code 425 is found.

The core commit id needs to be bumped for this feat. to come in OCIS. I can do this.

[individual-it]

@kiranparajuli589 I think we need an other wait here, I don't think we will see a 425 when checking the etag of a parent folder. I think we need an other waiting thing when comparing the etags

[kiranparajuli589]

sorry, accidentally closed with #4129

[SwikritiT]

new failure in: https://drone.owncloud.com/owncloud/ocis/13192/47/6

Scenario Outline: as share receiver copying a file inside a folder changes its etag for all collaborators      # /srv/app/testrunner/tests/acceptance/features/apiWebdavEtagPropagation2/copyFileFolder.feature:153
    Given user "Brian" has been created with default attributes and without skeleton files                       # FeatureContext::userHasBeenCreatedWithDefaultAttributesAndWithoutSkeletonFiles()
    And the administrator has set the default folder for received shares to "Shares"                             # OccContext::theAdministratorHasSetTheDefaultFolderForReceivedSharesTo()
    And parameter "shareapi_auto_accept_share" of app "core" has been set to "no"                                # AppConfigurationContext::serverParameterHasBeenSetTo()
    And using <dav_version> DAV path                                                                             # FeatureContext::usingOldOrNewDavPath()
    And user "Alice" has created folder "/upload"                                                                # FeatureContext::userHasCreatedFolder()
    And user "Alice" has uploaded file with content "uploaded content" to "/upload/file.txt"                     # FeatureContext::userHasUploadedAFileWithContentTo()
    And user "Alice" has shared folder "/upload" with user "Brian"                                               # FeatureContext::userHasSharedFileWithUserUsingTheSharingApi()
    And user "Brian" has accepted share "/upload" offered by user "Alice"                                        # FeatureContext::userHasReactedToShareOfferedBy()
    And user "Alice" has stored etag of element "/"                                                              # WebDavPropertiesContext::userHasStoredEtagOfElement()
    And user "Alice" has stored etag of element "/upload"                                                        # WebDavPropertiesContext::userHasStoredEtagOfElement()
    And user "Alice" has stored etag of element "/upload/file.txt"                                               # WebDavPropertiesContext::userHasStoredEtagOfElement()
    And user "Alice" has stored etag of element "/upload/file.txt" on path "/upload/renamed.txt"                 # WebDavPropertiesContext::userStoresEtagOfElementOnPath()
    And user "Brian" has stored etag of element "/"                                                              # WebDavPropertiesContext::userHasStoredEtagOfElement()
    And user "Brian" has stored etag of element "/Shares"                                                        # WebDavPropertiesContext::userHasStoredEtagOfElement()
    And user "Brian" has stored etag of element "/Shares/upload"                                                 # WebDavPropertiesContext::userHasStoredEtagOfElement()
    And user "Brian" has stored etag of element "/Shares/upload/file.txt"                                        # WebDavPropertiesContext::userHasStoredEtagOfElement()
    And user "Brian" has stored etag of element "/Shares/upload/file.txt" on path "/Shares/upload/renamed.txt"   # WebDavPropertiesContext::userStoresEtagOfElementOnPath()
    When user "Brian" copies file "/Shares/upload/file.txt" to "/Shares/upload/renamed.txt" using the WebDAV API # FeatureContext::userCopiesFileUsingTheAPI()
    Then the HTTP status code should be "201"                                                                    # FeatureContext::thenTheHTTPStatusCodeShouldBe()
    And these etags should have changed:                                                                         # WebDavPropertiesContext::theseEtagsShouldHaveChanged()
      | user  | path                       |
      | Alice | /                          |
      | Alice | /upload                    |
      | Alice | /upload/renamed.txt        |
      | Brian | /                          |
      | Brian | /Shares                    |
      | Brian | /Shares/upload             |
      | Brian | /Shares/upload/renamed.txt |
    And these etags should not have changed:                                                                     # WebDavPropertiesContext::theseEtagsShouldNotHaveChanged()
      | user  | path                    |
      | Alice | /upload/file.txt        |
      | Brian | /Shares/upload/file.txt |

    Examples:
      | dav_version |
      | old         |
      | new         |
      | spaces      |
        Expected stored etag to be some string but found null! (Exception)

[individual-it]

@kiranparajuli589 could you please provide some more details in here about what you have tried and what is the issue in your opinion

[kiranparajuli589]

At first, we thought the problem was with the asynchronous etag update.
So for the tests including the etag propagation:

• we added a retry (first 10 and then 20 iterations) if the etag is not yet updated.
• we added a delay of 1sec before every retry

But sadly,

Not a single test passed with the retry.

• failing tests fail even with retry and waits in between
• passing tests passed without a single retry

Tests runs and server logs can be found in this demostration PR: #4137

Therefore, the async behavior is not causing the test to be flaky.

[individual-it]

@butonic it looks to me that the async etag propagation is not really the issue, the etag of the parent does never update.

[kiranparajuli589]

TODO (not urgent, only come here if we see build failures related to this):

• skip all the tests including the etag comparisons for when running with the oCIS backend. ([tests-only] Skip the flaky etag propagation tests when running with oCIS CI core#40225)

[SagarGi]

@individual-it Build: https://drone.owncloud.com/owncloud/ocis/13691/48/7 failed today. shall we do it as mentioned by @kiranparajuli589 ?

[kiranparajuli589]

@individual-it Build: https://drone.owncloud.com/owncloud/ocis/13691/48/7 failed today. shall we do it as mentioned by @kiranparajuli589 ?

The falkyness is seen again :( As discussed in this scrum handover, we should proceed with this. Assigning myself to it.

[individual-it]

@kiranparajuli589 please skip the failing tests on ocis and then we need to hand this over to the development team, maybe @butonic

[kiranparajuli589]

Found an old issue addressing this owncloud/product#280, some tests were already skipped on ocis-oc-storage.

[kiranparajuli589]

closing on behalf of #4251