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

Undefined steps in acceptance tests should fail CI #5411

Closed
phil-davis opened this issue Jun 29, 2021 · 17 comments
Closed

Undefined steps in acceptance tests should fail CI #5411

phil-davis opened this issue Jun 29, 2021 · 17 comments
Labels
QA:team

Comments

@phil-davis
Copy link
Contributor

Example: https://drone.owncloud.com/owncloud/web/16897/49/10

  tests/acceptance/features/webUIDeleteFilesFolders/deleteFilesFolders.feature:144

    Given user "Alice" has been created with default attributes and without skeleton files
    And user "Alice" has logged in using the webUI
√ Element <input[autocomplete="kopano-account username"]> was visible after 914 milliseconds.
√ Element <input[autocomplete="kopano-account username"]> was not present after 373 milliseconds.
√ Element <.oc-app-container> was visible after 982 milliseconds.
    And the user has browsed to the files page
√ Element <//*[self::table[contains(@class, "files-table")] or self::div[contains(@class, "files-empty")] or self::div[contains(@class, "files-not-found")]]> was present after 62 milliseconds.
    Given user "Alice" has created file "lorem.txt"
    And the user has reloaded the current page of the webUI
    And user "Alice" has created a "normal" tag with name "lorem"
    ? undefined
    And user "Alice" has added tag "lorem" to file "/lorem.txt"
    ? undefined
    When the user browses to the tags page
    ? undefined
    And the user searches for tag "lorem" using the webUI
    ? undefined
    Then file "lorem.txt" should be listed on the webUI
    - skipped
    When the user deletes file "lorem.txt" using the webUI
    - skipped
    Then as "Alice" file "lorem.txt" should not exist
    - skipped
    And file "lorem.txt" should not be listed on the webUI
    - skipped
    When the user browses to the files page
    - skipped
    Then file "lorem.txt" should not be listed on the webUI
    - skipped

And that pipeline is using this expected-failures file:

Checking expected failures in /var/www/owncloud/web/tests/acceptance/expected-failures-with-ocis-server-ocis-storage.md

That scenario is listed in expected-failures. So maybe that is OK?

But maybe we should always fail the pipeline when some step(s) are undefined?
@dpakach
Copy link
Contributor

dpakach commented Jun 30, 2021

@phil-davis I think many of the undefined steps comes mostly from suites such as webUITags and webUIAdminSettings. these features are not available in web but the feature files were copied from core when we first constructed tests for web 2 years ago. Maybe we can delete those suites and copy them back when these features are available in web.

@phil-davis
Copy link
Contributor Author

Could do so. Or we make "dummy" step definitions just have a call to make the step really fail?

It would be nice if nightwatch can pre-process a scenario, and if any step definition is not found, then fail the scenario without executing any steps. That would avoid wasting time in CI, where it currently executes various Given steps then fails. That wastes time creating the users/groups and then deleting them again.

@jasson99 jasson99 self-assigned this Jun 30, 2021
@jasson99
Copy link
Contributor

jasson99 commented Jul 1, 2021 •
edited by saw-jan
Loading

The following tests have undefined steps:

1. webUIAdminSettings: Admin settings feature not implemented yet [Removed these tests]

Show tests

2. webUIComments: Comments in sidebar [REMOVED THESE TESTS]

Show tests

3. webUIDeleteFilesFolders: Tags page not implemented yet

4. webUIFiles:

5. webUIFilesActionMenu:

@jasson99
Copy link
Contributor

jasson99 commented Jul 1, 2021 •
edited
Loading

6. webUIFileDetails: Tags page not implemented yet

7. webUIFilesSearch: Tags page not implemented yet

8. webUIFilesSearch:

@jasson99
Copy link
Contributor

jasson99 commented Jul 1, 2021 •
edited
Loading

9. webUISharingAcceptShares:

10. webUISharingAcceptSharesToRoot:

@jasson99
Copy link
Contributor

jasson99 commented Jul 1, 2021 •
edited
Loading

11. webUISharingExternalToRoot: Might need to adjust test steps

12. webUISharingNotifications: Might need to adjust test steps

13. webUISharingNotificationsToRoot: Might need to adjust test steps

14. webUISharingPublicBasic: Might need to adjust test steps

15. webUISharingPublicManagement: Public link send by email field Might need to adjust test steps

16. webUISharingPublicManagement:

@jasson99
Copy link
Contributor

jasson99 commented Jul 1, 2021 •
edited
Loading

17. webUITags: Tags page not implemented yet [REMOVED THESE TESTS]

18. webUITrashbinDelete: impossible to navigate into a folder in the trashbin

19. webUIUpload:

20. webUIUpload:

21. webUIWebdavLockProtection

@jasson99 jasson99 mentioned this issue Jul 5, 2021
Merged
10 tasks
@jasson99 jasson99 removed their assignment Jul 9, 2021
@jasson99
Copy link
Contributor

jasson99 commented Jul 9, 2021

The tests related to AdminSettings, comments, and tags have been removed now. This is moved to backlog now, as we have planned to make some changes in nightwatch(if possible) such that the tests are parsed before they are run and whenever there is not stepdefinition, the tests fail early.

@phil-davis
Copy link
Contributor Author

I moved this out of the backlog. Can someone please report the status of this. It would be nice that if we accidentally write a typo in a feature file, that the undefined step causes the CI to fail - otherwise it is easy not to notice this kind of accidental error.

@pascalwengerter
Copy link
Contributor

I moved this out of the backlog. Can someone please report the status of this. It would be nice that if we accidentally write a typo in a feature file, that the undefined step causes the CI to fail - otherwise it is easy not to notice this kind of accidental error.

Pretty sure this is the status as of now? We've had this a bunch of times now with the middleware

@phil-davis
Copy link
Contributor Author

Pretty sure this is the status as of now? We've had this a bunch of times now with the middleware

I noticed this week the problem that if:

  • a scenario is already in expected-failures, and
  • someone refactors the scenario (for example, to change some Given step(s) to a form that uses the middleware) and
  • they make a typo in the step text

Then CI does not fail.

The scenario now fails in a different way - it has an undefined step. But the scenario is in expected-failures so the pipeline still "passes".

Maybe we can add a dry-run of all the acceptance test scenarios as a pipeline that happens "early" in CI. That could be done in a pipeline that runs at the same time as the e2e tests, or even earlier at the same time as other "lint"-related pipelines. That would catch undefined steps in feature files.

@phil-davis
Copy link
Contributor Author

We currently use nightwatch 1.7 and that does not seem to have a dry-run option:
https://nightwatchjs.org/guide/running-tests/command-line-options.html

nightwatch 2.0.0 beta is out: nightwatchjs/nightwatch#2939
It looks like dry-run support has been merged:
nightwatchjs/nightwatch#2962
nightwatchjs/nightwatch#2978
but only 2 days ago.

@individual-it do we:

  1. try to think of some other way to detect undefined steps
  2. wait and update to nightwatch v2 in a few months
  3. decide that this is a "won't fix" for the acceptance test infrastructure
  4. something else???

@phil-davis
Copy link
Contributor Author

We are currently pinned to nightwatch https://github.com/nightwatchjs/nightwatch/releases/tag/v1.7.11
nightwatch is up to release https://github.com/nightwatchjs/nightwatch/releases/tag/v2.1.7

@individual-it
What to do? Update to nightwatch v2 and then do something to address this issue? Close this issue and stay on nightwatch v1? Or?

@individual-it
Copy link
Member

I would suggest to have a short looks how difficult and how time-consuming it would be to update nightwatch. If it requires too much refactoring I would be reluctant to update because we don't want to focus on nightwatch tests in future

@kiranparajuli589 kiranparajuli589 self-assigned this Jun 15, 2022
@kiranparajuli589
Copy link
Contributor

kiranparajuli589 commented Jun 22, 2022 •
edited
Loading

Nightwatch v2

I invested some time playing around with version 2 of Nightwatch js.

Major changes:

  • No longer depends on nightwatch-api (https://github.com/mucsi96/nightwatch-api)
  • Integrated test runner for CucumberJs: We have to configure nightwatch.conf.js to use the cucumber test-runner. It has some more features like setting parallel test runs.
  • start/stop sessions are now different (previously used to be done with the nightwatch-api)

Needs more investigation

Although nightwatch-api is dropped, most of the element interaction APIs are the same. We just have to access them from a global var browser rather client.

Demo setup with nightwatch v2

Setup for a google search test automation (no source code, just test setup 😉). https://github.com/kiranparajuli589/nightwatch

Migration

In my opinion, just for the dry run feature, the version bump would be a little bit less reasonable. It's necessary to modify all step definitions, helpers, and page-objects so that they no longer use the nightwatch-api. This will not be a single pr task (This could be a long migration).

Some solutions:

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

@kiranparajuli589 kiranparajuli589 mentioned this issue Jun 23, 2022
Closed
@kiranparajuli589 kiranparajuli589 removed their assignment Jun 27, 2022
@SwikritiT
Copy link
Contributor

Since we've decided that we won't be making any changes in Nightwatch tests, let alone implementing new steps, this issue is now irrelevant. Now what we can do is remove all the scenarios that have unimplemented steps. I'll create a separate issue for that. I'm closing this issue.

cc @phil-davis @individual-it @kiranparajuli589

@SwikritiT
Copy link
Contributor

new issue to remove undefined steps is here: #7215

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
QA:team
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
7 participants
@phil-davis @individual-it @pascalwengerter @dpakach @kiranparajuli589 @jasson99 @SwikritiT