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

[tests-only] [full-ci] transfers occ command test scenarios to cli test suites #39443

Merged
merged 1 commit into from
Oct 14, 2022
Merged

[tests-only] [full-ci] transfers occ command test scenarios to cli test suites #39443

merged 1 commit into from
Oct 14, 2022

Conversation

sakshamgurung
Copy link
Contributor

@sakshamgurung sakshamgurung commented Nov 1, 2021 •
edited by kiranparajuli589
Loading

Description

  • Separate scenarios using occ commands as When steps to cli pipelines
  • Use occ commands as Given steps when used for setting up the scenario

Related Issue

Motivation and Context

How Has This Been Tested?

Screenshots (if appropriate):

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

@sakshamgurung sakshamgurung self-assigned this Nov 1, 2021
@sakshamgurung
Copy link
Contributor Author

sakshamgurung commented Nov 1, 2021 •
edited
Loading

@phil-davis @kiranparajuli589 occ command scenarios in listed files needs to run with make test-acceptance-webui command which means I can't run them under make test-acceptance-cli command. Should I leave these tests?

  • webUIPersonalSettings/accessToChangeFullNameThroughConfig.feature
  • webUIPersonalSettings/personalGeneralSettings.feature

@sakshamgurung sakshamgurung marked this pull request as ready for review November 1, 2021 09:11
@kiranparajuli589
Copy link
Contributor

kiranparajuli589 commented Nov 2, 2021 •
edited
Loading

@phil-davis @kiranparajuli589 occ command scenarios in listed files needs to run with make test-acceptance-webui command which means I can't run them under make test-acceptance-cli command. Should I leave these tests?

  • webUIPersonalSettings/accessToChangeFullNameThroughConfig.feature
  • webUIPersonalSettings/personalGeneralSettings.feature

@sakshamgurung, since the tests uses occ commands as when step, I think all these transferred tests should be moved inside cli suites, that should allow you to use make test-acceptance-cli too

@sakshamgurung sakshamgurung force-pushed the occ-test-transfer branch 2 times, most recently from f69d32d to 9a528ba Compare November 2, 2021 09:06
@sakshamgurung
Copy link
Contributor Author

New webUIWithCli folder is created to contain feature files that use both webui and cli contexts. Putting these files inside cli test suite don't allow the use of webui contexts and needs changes in drone and make files, so creating a separate folder inside webui test suite makes more ease.

phil-davis
phil-davis reviewed Nov 2, 2021
View reviewed changes
Copy link
Contributor

@phil-davis phil-davis left a comment

Choose a reason for hiding this comment

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

See my comments. We need to think about this for some (many?) of the test scenarios that really are about making sure that the API works.

tests/acceptance/features/cliMain/excludeGroupFromReceivingSharesToSharesFolder.feature Outdated Show resolved Hide resolved
@sakshamgurung sakshamgurung marked this pull request as draft November 3, 2021 11:29
@phil-davis phil-davis self-requested a review November 3, 2021 11:53
@sakshamgurung sakshamgurung marked this pull request as ready for review November 8, 2021 10:42
@sakshamgurung sakshamgurung force-pushed the occ-test-transfer branch 3 times, most recently from 29ef833 to f2845c6 Compare November 8, 2021 11:10
@sakshamgurung sakshamgurung force-pushed the occ-test-transfer branch 2 times, most recently from 212cd79 to 2d95452 Compare November 15, 2021 04:51
@phil-davis
Copy link
Contributor

phil-davis commented Nov 30, 2021 •
edited
Loading

Note: PR #39514 has the other pending test changes waiting for oC10 master.

I will look at this PR later in the week and sort out some refactoring related to having the new sysconfig endpoint in the testing app: owncloud/testing#190

We can use that for system settings changes where the occ config:system:set command is being used just as a Given step in scenario setup.

This PR is no longer in a current project, because the code is "essentially done". I assigned myself so that I find it easily.

@sonarqubecloud
Copy link

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
Copy link
Contributor

Note: there is no rush for this. I would like to sort out some things. I am away this coming week and will look at it the first week of 2022.

@phil-davis phil-davis force-pushed the occ-test-transfer branch from 6f7756e to 9981464 Compare June 6, 2022 09:22
@kiranparajuli589
Copy link
Contributor

I'll see what we can do on this.

@kiranparajuli589 kiranparajuli589 force-pushed the occ-test-transfer branch 4 times, most recently from 0df052d to 29092e2 Compare October 11, 2022 07:36
@kiranparajuli589 kiranparajuli589 marked this pull request as ready for review October 11, 2022 10:17
@individual-it
Copy link
Member

CI failes with

+ php occ security:certificates:import /drone/server.crt
certificate not found

also there is more than only moving the tests happening here, please adjust original description

@sakshamgurung @kiranparajuli589
manage scenarios using occ commands used as the when (transfer to CLI…
2c8735e 
… suites) and given (involving in setting up) steps
@sonarqubecloud
Copy link

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

saw-jan
saw-jan reviewed Oct 13, 2022
View reviewed changes
Copy link
Member

@saw-jan saw-jan left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@individual-it individual-it merged commit a3b7e2c into master Oct 14, 2022
@delete-merged-branch delete-merged-branch bot deleted the occ-test-transfer branch October 14, 2022 07:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@saw-jan saw-jan saw-jan left review comments

@individual-it individual-it individual-it approved these changes

@phil-davis phil-davis Awaiting requested review from phil-davis

@kiranparajuli589 kiranparajuli589 Awaiting requested review from kiranparajuli589

@SwikritiT SwikritiT Awaiting requested review from SwikritiT

@SagarGi SagarGi Awaiting requested review from SagarGi

Assignees

@kiranparajuli589 kiranparajuli589

Labels
QA:team
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@sakshamgurung @kiranparajuli589 @individual-it @phil-davis @SwikritiT @SagarGi @saw-jan