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

Fix node tests ci #32591

Merged
merged 1 commit into from
May 30, 2022
Merged

Fix node tests ci #32591

merged 1 commit into from
May 30, 2022

Conversation

CarlSchwan
Copy link
Member

  • Update jest snapshot
  • Fix importing 3rdparty vue components
  • Disable temporarely tests using nextcloud-vue/Button

@CarlSchwan CarlSchwan added bug 3. to review Waiting for reviews labels May 25, 2022
@CarlSchwan CarlSchwan self-assigned this May 25, 2022
PVince81
PVince81 reviewed May 25, 2022
View reviewed changes
jest.config.js Outdated Show resolved Hide resolved
PVince81
PVince81 reviewed May 25, 2022
View reviewed changes
Copy link
Member

@PVince81 PVince81 left a comment

Choose a reason for hiding this comment

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

Lint is failing

apps/dav/src/views/CalDavSettings.spec.js Outdated Show resolved Hide resolved
@CarlSchwan CarlSchwan force-pushed the fix/node-test branch 2 times, most recently from 277431d to 122a3fd Compare May 27, 2022 13:54
@MichaIng MichaIng mentioned this pull request May 28, 2022
Merged
@MichaIng
Copy link
Member

MichaIng commented May 28, 2022 •
edited
Loading

Looks like the new file needs to be added to tests to satisfy Drone checkers:

ERROR: There were 1 additional files:
jest.config.js
ERROR: Please remove or add those files again or inform the release team about those now files to be included or excluded from the release tar ball.

@CarlSchwan
Fix node tests ci
623293c 
- Update jest snapshot
- Fix importing 3rdparty vue components
- Disable temporarely tests using nextcloud-vue/Button

Signed-off-by: Carl Schwan <carl@carlschwan.eu>
PVince81
PVince81 reviewed May 30, 2022
View reviewed changes
apps/dav/src/views/CalDavSettings.spec.js
@@ -76,6 +74,9 @@ describe('CalDavSettings', () => {
)
expect(sendEventRemindersPush).toBeChecked()

/*
FIXME userEvent.click is broken with nextcloud-vue/Button
Copy link
Member

Choose a reason for hiding this comment

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

link to ticket ?

@PVince81 PVince81 requested review from skjnldsv, artonge and Pytal May 30, 2022 09:48
PVince81
PVince81 reviewed May 30, 2022
View reviewed changes
jest.config.js

// TODO: find a way to consolidate this in one place, with webpack.common.js
const ignorePatterns = [
'vue-material-design-icons',
Copy link
Member

Choose a reason for hiding this comment

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

hope we can eventually centralize those as this is copied from the talk repo

and also wondering how many we still need now that IE / old Edge support is out

PVince81
PVince81 approved these changes May 30, 2022
View reviewed changes
Copy link
Member

@PVince81 PVince81 left a comment

Choose a reason for hiding this comment

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

👍 see comments

@PVince81 PVince81 mentioned this pull request May 30, 2022
Merged
@PVince81 PVince81 merged commit 0886437 into master May 30, 2022
@PVince81 PVince81 deleted the fix/node-test branch May 30, 2022 15:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@artonge artonge artonge approved these changes

@PVince81 PVince81 PVince81 approved these changes

@skjnldsv skjnldsv Awaiting requested review from skjnldsv

@Pytal Pytal Awaiting requested review from Pytal

Assignees

@CarlSchwan CarlSchwan

Labels
3. to review Waiting for reviews bug
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@CarlSchwan @MichaIng @PVince81 @artonge