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]Adjust tests according to the expected behavior in OCIS #39111

Merged
merged 1 commit into from
Aug 19, 2021

Conversation

jasson99
Copy link
Contributor

@jasson99 jasson99 commented Aug 13, 2021

Description

This PR adds notToImplementOnOCIS tag for the test scenarios in which the folders are to be moved/renamed from the shares folder, which are never to be implemented on ocis. For some scenarios where some changes might reproduce the expected behavior of that in ocis, required adjustments are made. Few scenarios are added for the behavior only in ocis.

Related Issue

How Has This Been Tested?

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Database schema changes (next release will require increase of minor version instead of patch)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Technical debt
  • Tests only (no source changes)

Checklist:

  • Code changes
  • Unit tests added
  • Acceptance tests added
  • Documentation ticket raised:
  • Changelog item, see TEMPLATE

@jasson99 jasson99 self-assigned this Aug 13, 2021
@jasson99 jasson99 force-pushed the adjustMoveSharesTestsInOcis branch 2 times, most recently from cdc7491 to 3944c7f Compare August 17, 2021 10:36
@phil-davis phil-davis self-requested a review August 18, 2021 10:20
Copy link
Contributor

@Talank Talank left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@@ -63,6 +63,41 @@ Feature: sharing
| 1 | 100 |
| 2 | 200 |


@issue-ocis-1289 @skipOnOcV10
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why do we skip this on oc10?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have removed the skipOnOc tag now. I had skipped it thinking the scenarios were almost same, but there is no harm in running in oc10 as the test passes there.

And user "Alice" has uploaded file "filesForUpload/textfile.txt" to "/textfile0.txt"
And user "Alice" has shared file "textfile0.txt" with group "grp1"
And user "Brian" has accepted share "/textfile0.txt" offered by user "Alice"
And user "Brian" has created folder "/FOLDER"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

that folder is never used for anything in the test

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, it was a mistake. I have fixed it now.

@@ -63,6 +63,41 @@ Feature: sharing
| 1 | 100 |
| 2 | 200 |


@issue-ocis-1289 @skipOnOcV10
Scenario Outline: keep group permissions in sync
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't understand what we are trying to test here, could you please explain

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the above scenario actually tests that when a share which is moved to another folder by the receiver, and then when the sharer updates the share permission, the share permission is updated as expected. But since moving the share outside a shares folder, or even creating a new folder inside shares folder is not going to be implemented in ocis, I recreated a similar scenario where the share is just renamed instead of trying to move to some other folder.

@files_sharing-app-required
@issue-ocis-reva-34
@files_sharing-app-required @issue-ocis-reva-34
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do we really want to do that? That will mess up more lines in the expected to fail file

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, the changes has been made in expected failures, so while I am in this file, I wanted to make it uniform

@sonarcloud
Copy link

sonarcloud bot commented Aug 19, 2021

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
No Duplication information No Duplication information

@phil-davis phil-davis merged commit 5822d54 into master Aug 19, 2021
@delete-merged-branch delete-merged-branch bot deleted the adjustMoveSharesTestsInOcis branch August 19, 2021 07:33
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.

6 participants