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

Polyfill Vue Test Utils 2 API #8103

Merged
merged 98 commits into from Dec 15, 2022
Merged

Polyfill Vue Test Utils 2 API #8103

merged 98 commits into from Dec 15, 2022

Conversation

dschmidt
Copy link
Member

@dschmidt dschmidt commented Dec 12, 2022 •
edited

Description

This PR polyfills the new Vue Test Utils 2 Api which we need for Vue 3 (compat mode) and ports our tests to use them.

It gets rid of vuex-mock-store and basically also of vuex-extensions (but we left that one in at runtime, because we didn't want to risk any breaking change at runtime in stable)

Related Issue

  • Fixes <issue_link>

Motivation and Context

We want to switch to Vue 3 (compat mode) in master sooner or later. Adjusting our test suite first should keep the scope way more limited.
Doing this in the stable branch will help keeping tests in stable and merging them to master and will also make it much easier to maintain them for our stable release.

How Has This Been Tested?

just test changes :trollface:

Screenshots (if appropriate):

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Technical debt
  • Tests

Checklist:

  • Code changes
  • Unit tests added
  • Acceptance tests added
  • Documentation ticket raised:

Followup tasks:

  • Merge to master after merging
  • Port remaining test in user management app in master (it has changed a lot in master during the Vue 2.7 port, so it didn't make sense to make changes in stable)
  • Check for other remaining/new tests that only exit on master that may need to be ported as well
  • Remove vuex-extensions completely
  • Avoid extending storeOptions, instead use proper mockImplementations for defaultStoreOptions as they are typesafer
  • Check if more defaultStubs or mocks can be used in all tests

@update-docs
Copy link

update-docs bot commented Dec 12, 2022

Thanks for opening this pull request! The maintainers of this repository would appreciate it if you would create a changelog item based on your changes.

@ownclouders
Copy link
Contributor

ownclouders commented Dec 12, 2022 •
edited

Results for oCISFiles3 https://drone.owncloud.com/owncloud/web/30647/58/1
💥 The oCISFiles3 tests pipeline failed. The build has been cancelled.

@ownclouders
Copy link
Contributor

Results for oC10SharingExternalRoot https://drone.owncloud.com/owncloud/web/30647/43/1
💥 The oC10SharingExternalRoot tests pipeline failed. The build has been cancelled.

@ownclouders
Copy link
Contributor

Results for oC10SharingExternal https://drone.owncloud.com/owncloud/web/30647/42/1
💥 The oC10SharingExternal tests pipeline failed. The build has been cancelled.

@ownclouders
Copy link
Contributor

Results for oC10FileFolderOperations https://drone.owncloud.com/owncloud/web/30647/14/1
💥 The oC10FileFolderOperations tests pipeline failed. The build has been cancelled.

@ownclouders
Copy link
Contributor

Results for oC10RenameFileFolder https://drone.owncloud.com/owncloud/web/30647/15/1
💥 The oC10RenameFileFolder tests pipeline failed. The build has been cancelled.

@JammingBen JammingBen force-pushed the test-compat branch 3 times, most recently from 46f0179 to 06aa74a Compare December 13, 2022 15:59
dschmidt and others added 19 commits December 14, 2022 09:14
@dschmidt
Polyfill Vue Test Utils 2 API
323ed31
@JammingBen @dschmidt
Use new test syntax for SharesNavigation component
710e657
@JammingBen @dschmidt
Use new test syntax for CreateAndUpload component
a2dd7b0
@JammingBen @dschmidt
Use new test syntax for CreateSpace component
06ddeae
@JammingBen @dschmidt
Use new test syntax for SidebarToggle component
2cfc2b4
@JammingBen @dschmidt
Use new test syntax for ViewOptions component
4a60fd0
@JammingBen @dschmidt
Use new test syntax for BatchActions component
1c00626
@JammingBen @dschmidt
Use new test syntax for ResourceUpload component
416ecfc
@JammingBen @dschmidt
Use new test syntax for ContextActions component
0d32b02
@JammingBen @dschmidt
Use new test syntax for ListInfo component
5940043
@JammingBen @dschmidt
Use new test syntax for NotFoundMessage component
fe8ed3e
@JammingBen @dschmidt
Use new test syntax for Pagination component
917120b
@JammingBen @dschmidt
Use new test syntax for QuickActions component
d89026a
@JammingBen @dschmidt
Use new test syntax for ResourceTable component
2b4073e
@JammingBen @dschmidt
Use new test syntax for List component
3e7c2a2
@JammingBen @dschmidt
Use new test syntax for Preview component
12a4374
@JammingBen @dschmidt
Use new test syntax for QuotaModal component
ddfcd71
@JammingBen @dschmidt
Use translations in defaultComponentMocks
a353263
@JammingBen @dschmidt
Add mock Axios test helper
59c64f9
@JammingBen JammingBen mentioned this pull request Dec 14, 2022
Merged
5 tasks
@dschmidt
Remove some superfluous duplicated await nextTicks
5a95cee
@dschmidt
Use defaultStubs in SidebarNav.spec.ts
6970d53
@dschmidt
Sort defaultStubs
c334b84
@dschmidt
Get rid of tests/unit/stubs/index.js
42f9092
@dschmidt
Simplify web-test-helpers imports
e7de287
@dschmidt
Avatar: handle 404 silently
8855eb0
@dschmidt
Remove web-test-helpers as dependencies
3440822 
Tests are not run from their respective packages, so we do not need to depend on web-test-helpers in the packages (and especially not in runtime deps)
@dschmidt
Do not mock out vue-gettext in DeleteUserModal
ed28e84 
The test is imho more expressive if we check the value passed into the component is actually used instead of just returning a placeholder from a mock.
@dschmidt
Do not mock out vue-gettext in DeleteGroupModal
304cc13 
Same reasoning as for DeleteUserModal before: test is more expressive when the output actually differs based on the input
@dschmidt
Fix lint
d248ae2
@sonarcloud
Copy link

sonarcloud bot commented Dec 14, 2022

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 1 Code Smell

92.2% 92.2% Coverage
0.0% 0.0% Duplication

kulmann
kulmann reviewed Dec 15, 2022
View reviewed changes
packages/web-app-files/tests/unit/components/SideBar/FileInfo.spec.ts
describe('FileInfo', () => {
it('shows file info', () => {
const { wrapper } = createWrapper()
// FIXME check for highlightedFile
Copy link
Member

Choose a reason for hiding this comment

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

Is this a followup-fixme?

Copy link
Collaborator

Choose a reason for hiding this comment

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

This test needs to be reworked in general, but that's a follow-up. The comment is not really telling that, yeah 😅

kulmann
kulmann approved these changes Dec 15, 2022
View reviewed changes
Copy link
Member

@kulmann kulmann left a comment

Choose a reason for hiding this comment

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

I don't have proper words for this awesomeness. Good job guys, thank you! ❤️

JammingBen
JammingBen approved these changes Dec 15, 2022
View reviewed changes
Copy link
Collaborator

@JammingBen JammingBen left a comment

Choose a reason for hiding this comment

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

:trollface:

@JammingBen JammingBen merged commit e35a099 into stable-6.0 Dec 15, 2022
@delete-merged-branch delete-merged-branch bot deleted the test-compat branch December 15, 2022 10:40
ownclouders pushed a commit that referenced this pull request Dec 15, 2022
@JammingBen
commit e35a099
8c54f1d 
Merge: 6cace5c d248ae2
Author: Jannik Stehle <50302941+JammingBen@users.noreply.github.com>
Date:   Thu Dec 15 11:40:46 2022 +0100

    Merge pull request #8103 from owncloud/test-compat

    Polyfill Vue Test Utils 2 API
@dschmidt dschmidt mentioned this pull request Dec 15, 2022
Merged
10 tasks
ownclouders pushed a commit that referenced this pull request Dec 16, 2022
@JammingBen
commit e35a099
67f209f 
Merge: 6cace5c d248ae2
Author: Jannik Stehle <50302941+JammingBen@users.noreply.github.com>
Date:   Thu Dec 15 11:40:46 2022 +0100

    Merge pull request #8103 from owncloud/test-compat

    Polyfill Vue Test Utils 2 API
ownclouders pushed a commit that referenced this pull request Dec 16, 2022
@JammingBen
commit e35a099
3507869 
Merge: 6cace5c d248ae2
Author: Jannik Stehle <50302941+JammingBen@users.noreply.github.com>
Date:   Thu Dec 15 11:40:46 2022 +0100

    Merge pull request #8103 from owncloud/test-compat

    Polyfill Vue Test Utils 2 API
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kulmann kulmann kulmann approved these changes

@JammingBen JammingBen JammingBen approved these changes

@lookacat lookacat Awaiting requested review from lookacat

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@dschmidt @ownclouders @lookacat @kulmann @JammingBen