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

New resharing concept breaks testsuite #3828

Closed
kobergj opened this issue May 18, 2022 · 11 comments
Closed

New resharing concept breaks testsuite #3828

kobergj opened this issue May 18, 2022 · 11 comments
Labels
Category:Change Change existing functionality Interaction:Needs-help Asking some hints to engineering when the issue can't be reproduced QA:p2 QA:team Type:Technical-Debt

Comments

@kobergj
Copy link
Collaborator

kobergj commented May 18, 2022

Describe the bug

With the new reshare concept some tests are failing. We need to discuss how we can adjust them (or if the reshare concept is faulty)
See cs3org/reva#2877

Steps to reproduce

With reva from cs3org/reva#2877 and master ocis run test mentioned in list

Expected behavior

The test should pass

Actual behavior

The tests fail

List

Additional context

Add any other context about the problem here.

@micbar
Copy link
Contributor

micbar commented May 19, 2022

@phil-davis @individual-it we need to work together on this. The main problem is that the bitmask for the permissions has 31 as a representation for all permissions which is not true anymore.

@micbar micbar added QA:team QA:p2 Type:Technical-Debt Category:Change Change existing functionality Interaction:Needs-help Asking some hints to engineering when the issue can't be reproduced and removed Type:Bug labels May 19, 2022
@kobergj
Copy link
Collaborator Author

kobergj commented May 19, 2022

@phil-davis @micbar @individual-it sorry this ticket is not ready yet. I'm still trying to figure out which tests need to be fixed. So nothing to do atm. Sorry for confusion

@kobergj
Copy link
Collaborator Author

kobergj commented May 20, 2022

Update: reva seems fine with the changes, we have just some spaces ocis test failing. They are explicitly testing that resharing does not work. So these need to be changed. See list of failing tests in ocis PR:
https://github.com/owncloud/ocis/pull/3804/files#diff-733a682a252e8a35d985692d58d7b793107aa0abe81eac800edd774e3415fc07R26-R34

@micbar
Copy link
Contributor

micbar commented May 20, 2022

@kobergj thanks!

I am fine with adding these tests to the expected failures now.

@ScharfViktor Can then evaulate changes to the gherkin steps later.

@ScharfViktor
Copy link
Contributor

Yes, sure. I'll do it

@ScharfViktor
Copy link
Contributor

ScharfViktor commented May 23, 2022

Previously we could share the project space with the role of manager, which corresponded to (shareType=7, permissions=31). editor was matched 15 permissions.

Now we don't differentiate between manager and editor via int permissions(manager=31 and editor=31). Do we need new int permissions for manager?

The main question is: do we want to allow a user with the editor role (perm=31) to share space/object with another user via share and via link? @micbar

Also, space sharing with the manager role does not work with the web. The web sends permissions=31 and gets that the user is an editor:
https://localhost:9200/graph/v1.0/me/drives?%24top=0&%24skip=0&%24orderby=name+asc&%24filter=driveType+eq+project
Api test is passed because it sends request (role = manager) instead permissions = 31

@kobergj
Copy link
Collaborator Author

kobergj commented May 23, 2022

Just small info: Web is already working on sending roles instead of int permissions. @fschade is that already merged?

@kobergj
Copy link
Collaborator Author

kobergj commented May 24, 2022

@individual-it @phil-davis

We found an issue with the reva test suite. You can see it in apiShareReshareToShares3/reShareUpdate.feature:153 for example.

We think the problematic test line is:
And user "Alice" has updated the last share of "Alice" with
The last share will send a request for the wrong share (here 4 instead of the correct 3)
It is probably related to int ids of reva, as the test works correct against ocis (which is using uuids)

For now this not blocking us, as tests get green anyways, but I have no clue how many other tests are affected.

cc @fschade @micbar

@phil-davis
Copy link
Contributor

Note: I am working on issue owncloud/core#40093 in PR owncloud/core#40095 - that aims to make all those "somebody does something related to the last share" steps work regardless of the format of share-ids. The test code will remember more of the shares that it created in a scenario, and will avoid ever having to do a "general API query" to discover the shares that exist - no more making a "partly-educated guess" about what "the last share" refers to.

@phil-davis
Copy link
Contributor

core PR owncloud/core#40114 is in review. Hopefully it all passes and can be merged overnight. Then I will bump the core-commit-id in reva and oCIS. That should help with test steps about "the last share"

@micbar
Copy link
Contributor

micbar commented Jul 4, 2022

Done

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Category:Change Change existing functionality Interaction:Needs-help Asking some hints to engineering when the issue can't be reproduced QA:p2 QA:team Type:Technical-Debt
Projects
Archived in project
Development

No branches or pull requests

4 participants