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-only] Remove test from expected to fail and bump commit id #2381

Merged
merged 1 commit into from
Dec 16, 2021

Conversation

SwikritiT
Copy link
Contributor

This PR removes the test scenario from the expected to fail file in accordance with the fixes made by PR owncloud/core#39598 and bumps the commit id as well.

@SwikritiT SwikritiT changed the title [tests-only] Fix expected to fail file edge [tests-only] Remove test from expected to fail and bump commit id Dec 15, 2021
@phil-davis
Copy link
Contributor

https://drone.cernbox.cern.ch/cs3org/reva/4730/8/6

  Scenario: send PUT requests to another user's webDav endpoints as normal user                             # /drone/src/tmp/testrunner/tests/acceptance/features/apiAuthWebDav/webDavPUTAuth.feature:40
    When user "Brian" requests these endpoints with "PUT" including body "doesnotmatter" about user "Alice" # OCSContext::userSendsRequestToTheseEndpointsWithBody()
      | endpoint                                       |
      | /remote.php/dav/files/%username%/textfile1.txt |
      | /remote.php/dav/files/%username%/PARENT        |
    Then the HTTP status code of responses on all endpoints should be "403"                                 # FeatureContext::theHTTPStatusCodeOfResponsesOnAllEndpointsShouldBe()
      Responses did not return expected http status code
      Failed asserting that 409 is identical to 403.
    When user "Brian" requests these endpoints with "PUT" including body "doesnotmatter" about user "Alice" # OCSContext::userSendsRequestToTheseEndpointsWithBody()
      | endpoint                                           |
      | /remote.php/dav/files/%username%/PARENT/parent.txt |
    Then the HTTP status code of responses on all endpoints should be "403"                                 # FeatureContext::theHTTPStatusCodeOfResponsesOnAllEndpointsShouldBe()

That is strange - PR #2380 to master branch has passed. But in edge branch it returns 409 even for the file and folder in the root.

On oCIS it returned 403 - all was good.

Needs investigation.

@wkloucek
Copy link
Contributor

On oCIS it returned 403 - all was good.

oCIS master still runs on REVA master, therefore this looks like a regression on edge.

I think there is no way around having two different expected failure files for REVA master / edge

@phil-davis
Copy link
Contributor

I think there is no way around having two different expected failure files for REVA master / edge

The same expected-failures file will have different contents in master and edge branches.

@wkloucek It would be nice to understand where this happened. It is not a problem on oCIS, so I guess that the difference is something that is waiting to be pulled from edge into oCIS?

@wkloucek
Copy link
Contributor

I think there is no way around having two different expected failure files for REVA master / edge

The same expected-failures file will have different contents in master and edge branches.

@wkloucek It would be nice to understand where this happened. It is not a problem on oCIS, so I guess that the difference is something that is waiting to be pulled from edge into oCIS?

@butonic @kobergj @aduffeck might know, since they are working on edge

@kobergj
Copy link
Contributor

kobergj commented Dec 16, 2021

@phil-davis we are still working on including edge branch into ocis. As of now it is still running on master.

edge contains the spaces registry, which is a huge change compared to master. It could be that this somehow leads to a different statuscode returned. If it is not critical I would prefer having different content in the expected failure file on edge and master for now.

@phil-davis
Copy link
Contributor

If it is not critical I would prefer having different content in the expected failure file on edge and master for now.

OK, then should I raise an issue about this "regression/difference" and, in expected-failures, link to the issue?

That would give the issue visibility.

But where to raise the issue? This is not a problem in cs3org/reva master so it seems a bit "unfair" to raise the issue there. And it is not an issue in owncloud/ocis

@kobergj
Copy link
Contributor

kobergj commented Dec 16, 2021

I see the problem. But we are working with highest prio on switching to edge branch for ocis. After this is completed it will be an issue for owncloud/ocis so we can raise the issue there. (Even if it is not 100% correct I would have no problem with having the issue raised before the switch is completed)

@wkloucek
Copy link
Contributor

But where to raise the issue?

We should still do it in oCIS. Basically there are three options:

  • issue is on expected failures on edge and master
  • issue is on expected failures on edge
  • issue is on expected failures on master

Depending on what branch oCIS uses these expected failures would also apply to oCIS.

So ideally we would track in the ticket where it does apply. But that's nothing for you and your team. I would say just continue creating tickets as of now. Developers can comment where it does apply

@phil-davis
Copy link
Contributor

I raised issue owncloud/ocis#2893 and linked it in expected-failures.

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.

4 participants