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

[full-ci][tests-only] Add tests for db-convert occ command #39393

Closed
wants to merge 8 commits into from

Conversation

saw-jan
Copy link
Member

@saw-jan saw-jan commented Oct 22, 2021

Description

Related Issue

Motivation and Context

How Has This Been Tested?

  • test environment:

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

@saw-jan saw-jan self-assigned this Oct 22, 2021
@ownclouders
Copy link
Contributor

💥 Acceptance tests pipeline cliDbConversion-sqlite-php7.4 failed. The build has been cancelled.

https://drone.owncloud.com/owncloud/core/33261/3/1

@saw-jan saw-jan force-pushed the db-occ-command branch 2 times, most recently from 90c1061 to 9ece122 Compare October 22, 2021 08:42
@ownclouders
Copy link
Contributor

💥 Acceptance tests pipeline cliDbConversion-sqlite-php7.4 failed. The build has been cancelled.

https://drone.owncloud.com/owncloud/core/33264/3/1

@saw-jan saw-jan force-pushed the db-occ-command branch 2 times, most recently from 43233d9 to fe3f449 Compare October 25, 2021 07:36
@ownclouders
Copy link
Contributor

💥 Acceptance tests pipeline cliDbConversion-sqlite-php7.4 failed. The build has been cancelled.

https://drone.owncloud.com/owncloud/core/33341/3/1

@ownclouders
Copy link
Contributor

💥 Acceptance tests pipeline cliDbConversion-sqlite-php7.4 failed. The build has been cancelled.

https://drone.owncloud.com/owncloud/core/33343/3/1

@ownclouders
Copy link
Contributor

💥 Acceptance tests pipeline cliDbConversion-sqlite-php7.4 failed. The build has been cancelled.

https://drone.owncloud.com/owncloud/core/33348/3/1

@ownclouders
Copy link
Contributor

💥 Acceptance tests pipeline cliDbConversion-sqlite-php7.4 failed. The build has been cancelled.

https://drone.owncloud.com/owncloud/core/33349/3/1

@saw-jan saw-jan changed the title [tests-only] Add tests for db-convert occ command [full-ci][tests-only] Add tests for db-convert occ command Oct 25, 2021
@saw-jan saw-jan force-pushed the db-occ-command branch 12 times, most recently from ed70889 to cac0c11 Compare November 1, 2021 09:02
@saw-jan
Copy link
Member Author

saw-jan commented Nov 8, 2021

Currently blocked due to unknown error in afterScenario hooks

@kiranparajuli589
Copy link
Contributor

I've initialized a state variable in featureContext to check if running for DB conversion. The variable state is set to true after successful DB conversion. but for the before scenario we don't know yet if the scenario is for DB conversion.

although multiple after hooks can be implemented even for specific tags, but the same is not true for a before. It can only be one and doesn't filter with tags.

Should we use some test environment variable, so that we can test some edge cases in this DB conversion feature?

cc @saw-jan @individual-it @phil-davis

@phil-davis
Copy link
Contributor

https://drone.owncloud.com/owncloud/core/33955/4/14
Notice: Array to string conversion in /drone/src/tests/TestHelpers/SetupHelper.php line 920

I have a fix for that in bf96d4a

You can cherry-pick that commit into this branch. Then you should get the real Exception message logged.

@kiranparajuli589
Copy link
Contributor

https://drone.owncloud.com/owncloud/core/33957/4/14

      ╳  Response is not valid XML after executing 'occ app:list --output json --no-ansi'. Is the testing app installed and enabled?

after a successful dB conversion, the testing app is disabled, maybe there is a reset.
but actually this was not true for me while trying locally.

cc @phil-davis

@phil-davis
Copy link
Contributor

That is in an AfterScenario. For this special test, we only want to run a single scenario. So we can skip any AfterScenario actions. Basically, we throw-away the system-under-test after all the Scenario Then steps have finished.
(If someone runs this db-convert scenario locally, it is going to expect that their existing install starts with sqlite and will convert them to mysql, assuming they have a mysql database ready etc, so it is not the sort of test that anyone can easily run locally anyhow, so cleanup after the test is not important)

So add whatever code and/or tags are needed to avoid running stuff in AfterScenario that causes fails.

@sonarqubecloud
Copy link

sonarqubecloud bot commented Dec 9, 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

@kiranparajuli589
Copy link
Contributor

That is in an AfterScenario. For this special test, we only want to run a single scenario. So we can skip any AfterScenario actions. Basically, we throw-away the system-under-test after all the Scenario Then steps have finished. (If someone runs this db-convert scenario locally, it is going to expect that their existing install starts with sqlite and will convert them to mysql, assuming they have a mysql database ready etc, so it is not the sort of test that anyone can easily run locally anyhow, so cleanup after the test is not important)

So add whatever code and/or tags are needed to avoid running stuff in AfterScenario that causes fails.

@phil-davis suggested changes have been implemented. Please review again.

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.

LGTM. I will cherry-pick and squash this onto the "pending acceptance tests" PR #39514

@phil-davis
Copy link
Contributor

cherry-picked to 5f98837

@phil-davis phil-davis closed this Dec 10, 2021
@saw-jan saw-jan deleted the db-occ-command branch March 11, 2022 11:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

add cli tests for occ db commands
4 participants