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

test: reduce visual regression tests flakiness #239

Merged
merged 2 commits into from
Apr 21, 2021

Conversation

web-padawan
Copy link
Member

Description

It turns out the openSubMenus helper was not using promises and the test was apparently passing or failing randomly, most likely depending on the network latency (in some cases, the screenshot was taken before last menu was opened).

Fixes #238

Type of change

  • Internal change

Copy link
Contributor

@sissbruecker sissbruecker left a comment

Choose a reason for hiding this comment

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

Code changes look good, needs checking why the visual tests fail

@web-padawan
Copy link
Member Author

It turns out that vaadin-menu-bar tests have failed because of font rendering issues. Example:

ltr-opened-diff

This is a known problem that happened in the past with individual repositories.
So there we actually had to fix browser version instead of latest.

It's unclear why the threshold in the diffOptions does not help, as in theory it should prevent this.

@sissbruecker
Copy link
Contributor

It's unclear why the threshold in the diffOptions does not help, as in theory it should prevent this.

Hmm, looking at node_modules/@web/test-runner-visual-regression/src/visualDiffCommand.ts it seems to not respect the diffOptions at all:

  const passed = diffPercentage === 0;

  if (!passed) {
    await saveDiff();
  }

  if (!passed || options.buildCache) {
    await saveFailed();
  }

  return {
    errorMessage: !passed
      ? `Visual diff failed. New screenshot is ${diffPercentage.toFixed(2)} % different.`
      : undefined,
    diffPercentage: -1,
    passed,
  };

@web-padawan web-padawan force-pushed the test/context-menu-visual-test branch from 011f002 to 2df66ce Compare April 21, 2021 11:58
@sonarcloud
Copy link

sonarcloud bot commented Apr 21, 2021

SonarCloud Quality Gate failed.

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
30.0% 30.0% Duplication

wtr-utils.js Show resolved Hide resolved
@web-padawan web-padawan changed the title test: fix context-menu items flaky test test: reduce visual regression tests flakiness Apr 21, 2021
@web-padawan web-padawan merged commit fb990d7 into master Apr 21, 2021
@web-padawan web-padawan deleted the test/context-menu-visual-test branch April 21, 2021 12:17
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.

Nested items context-menu visual test is broken
3 participants