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

test: move helpers from common.js to helpers.js #5368

Merged
merged 1 commit into from
Jan 24, 2023
Merged

test: move helpers from common.js to helpers.js #5368

merged 1 commit into from
Jan 24, 2023

Conversation

vursen
Copy link
Contributor

@vursen vursen commented Jan 24, 2023 •
edited
Loading

Description

Moved test helpers from common.js to helpers.js to make it consistent across the packages.

The basic rule:

  • common.js should be used for sharing global code with side effects e.g. global styles for visual tests.
  • helpers.js should be used for sharing helper functions.

Note
The PR targets 23.3 and 23.2 to keep automatic cherry-picks working.

Type of change

  • Internal

@vursen vursen marked this pull request as ready for review January 24, 2023 08:56
@vursen vursen requested a review from web-padawan January 24, 2023 08:56
@sonarqubecloud
Copy link

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

No Coverage information No Coverage information
0.0% 0.0% Duplication

web-padawan
web-padawan approved these changes Jan 24, 2023
View reviewed changes
Copy link
Member

@web-padawan web-padawan left a comment

Choose a reason for hiding this comment

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

LGTM 👍

packages/field-highlighter/test/visual/common.js
{ id: 'c', name: 'Admin', colorIndex: 2, fieldIndex: 0 },
];

export const setUsers = (field) => {
Copy link
Member

Choose a reason for hiding this comment

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

It's a great idea to separate common.js and helpers.js in this case 💪

@web-padawan web-padawan merged commit b0ff100 into master Jan 24, 2023
@web-padawan web-padawan deleted the test/rename-common-to-helpers branch January 24, 2023 09:01
@vaadin-bot
Copy link
Collaborator

Hi @vursen , this commit cannot be picked to 23.3 by this bot, can you take a look and pick it manually?
Error Message: Error: Command failed: git cherry-pick b0ff100
error: could not apply b0ff100... test: move helpers from common.js to helpers.js (#5368)
hint: After resolving the conflicts, mark them with
hint: "git add/rm ", then run
hint: "git cherry-pick --continue".
hint: You can instead skip this commit with "git cherry-pick --skip".
hint: To abort and get back to the state before "git cherry-pick",
hint: run "git cherry-pick --abort".

@vaadin-bot
Copy link
Collaborator

Hi @vursen , this commit cannot be picked to 23.2 by this bot, can you take a look and pick it manually?
Error Message: Error: Command failed: git cherry-pick b0ff100
error: could not apply b0ff100... test: move helpers from common.js to helpers.js (#5368)
hint: After resolving the conflicts, mark them with
hint: "git add/rm ", then run
hint: "git cherry-pick --continue".
hint: You can instead skip this commit with "git cherry-pick --skip".
hint: To abort and get back to the state before "git cherry-pick",
hint: run "git cherry-pick --abort".

@vursen vursen mentioned this pull request Jan 24, 2023
Merged
1 task
web-padawan pushed a commit that referenced this pull request Jan 24, 2023
@vursen
test: move helpers from common.js to helpers.js (#5368) (#5370)
0110946
@vursen vursen mentioned this pull request Jan 24, 2023
Merged
1 task
vursen added a commit that referenced this pull request Jan 24, 2023
@vursen
test: move helpers from common.js to helpers.js (#5368) (#5372)
43b1396
@vaadin-bot
Copy link
Collaborator

This ticket/PR has been released with Vaadin 24.0.0.alpha10 and is also targeting the upcoming stable 24.0.0 version.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@web-padawan web-padawan web-padawan approved these changes

Assignees
No one assigned
Labels
need to pick manually 23.2 need to pick manually 23.3 prerelease version for Vaadin 24.0.0 Released with Vaadin 24.0.0.alpha10 target/23.2 target/23.3
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@vursen @vaadin-bot @web-padawan