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][ui tests] Switch yarn to pnpm for oC Web tests #4878

Merged
merged 1 commit into from
Oct 24, 2022
Merged

Conversation

dschmidt
Copy link
Member

@dschmidt dschmidt commented Oct 21, 2022

Description

This PR makes drone use pnpm for running e2e and acceptance tests for oC Web, as we changed to pnpm there recently and we need to use the same lock file here as in the main repo.

As the .pnpm-store lives in the main folder of oC Web, we do not need separate caches for acceptance and e2e tests anymore. This simplifies the drone pipelines a bit.

We could also switch idp and settings services over to using pnpm but as that is more disruptive for go developers, I'm not doing that in this PR but will send a followup for that.

Related Issue

Motivation and Context

oC Web switched to pnpm and we wanna share the same lock files.

How Has This Been Tested?

  • test environment:
  • test case 1:
  • test case 2:
  • ...

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 only (no source changes)

Checklist:

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

Todo:

  • Fix all tests
  • Validate .pnpm-store is actually used
  • Decide whether we want to switch only oC Web tests to pnpm (required for [full-ci] Switch to PNPM web#7835) or also switch services/idp or services/settings

@update-docs
Copy link

update-docs bot commented Oct 21, 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.

@fschade
Copy link
Contributor

fschade commented Oct 21, 2022

Good decision, loving it 🤗

@dschmidt
Copy link
Member Author

I would like to evaluate what's needed to completely switch.
Afterwards we can discuss whether we want to fully switch for settings and idp apps as well or just for e2e/acceptance tests from web.

@@ -2,7 +2,7 @@ Bugfix: Fix konnectd build

Tags: konnectd

We fixed the default config for konnectd and updated the Makefile to include the `yarn install`and `yarn build` steps if the static assets are missing.
We fixed the default config for konnectd and updated the Makefile to include the `pnpm install`and `pnpm build` steps if the static assets are missing.
Copy link
Contributor

Choose a reason for hiding this comment

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

changelogs should not be touched

Copy link
Member Author

Choose a reason for hiding this comment

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

Right 😅

@saw-jan
Copy link
Member

saw-jan commented Oct 24, 2022

I will look at the CI failure.

@saw-jan
Copy link
Member

saw-jan commented Oct 24, 2022

Tests are now passing for settingsUI. Fixed has been applied by @dschmidt.
e2e test is still failing and needs a new web release. Currently, the test is failing due to the latest e2e test code running against old web build.

@dschmidt dschmidt changed the title [full-ci][ui tests] Switch yarn to pnpm [full-ci][ui tests] Switch yarn to pnpm for oC Web tests Oct 24, 2022
@dschmidt dschmidt marked this pull request as ready for review October 24, 2022 15:43
@sonarcloud
Copy link

sonarcloud bot commented Oct 24, 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 0 Code Smells

No Coverage information No Coverage information
No Duplication information No Duplication information

@dschmidt
Copy link
Member Author

Ready to merge from my end.

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.

Grrrrreat 🐯

@kulmann kulmann merged commit c818d71 into master Oct 24, 2022
ownclouders pushed a commit that referenced this pull request Oct 24, 2022
Merge: 50d97d6 966a775
Author: Benedikt Kulmann <benedikt@kulmann.biz>
Date:   Mon Oct 24 20:26:48 2022 +0200

    Merge pull request #4878 from owncloud/pnpm

    [full-ci][ui tests] Switch yarn to pnpm for oC Web tests
ownclouders pushed a commit that referenced this pull request Oct 25, 2022
Merge: 50d97d6 966a775
Author: Benedikt Kulmann <benedikt@kulmann.biz>
Date:   Mon Oct 24 20:26:48 2022 +0200

    Merge pull request #4878 from owncloud/pnpm

    [full-ci][ui tests] Switch yarn to pnpm for oC Web tests
@micbar micbar mentioned this pull request Nov 25, 2022
73 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants