Skip to content
This repository has been archived by the owner on Apr 18, 2024. It is now read-only.

ci: LEAP-753: E2E image problems #1708

Merged
merged 11 commits into from
Mar 5, 2024
Merged

Conversation

hlomzik
Copy link
Collaborator

@hlomzik hlomzik commented Feb 23, 2024

Various fixes for E2E tests. Main thing is to use less 3rd-party images in tests, so one specific image was replaced and all tests affected had to be adjusted because of new dimensions. Important other changes:

  • calculations and FFs fixed for Image transform test
  • new line chars fixed for a lot of tests (CRLF -> LF)
  • linting fixed for all tests
  • unnecessary waits removed in some tests

It's better to check the diff with whitespace changes hidden.

PR fulfills these requirements

  • Tests for the changes have been added/updated
  • Docs have been added/updated
  • Best efforts were made to ensure docs/code are concise and coherent (checked for spelling/grammatical errors, commented out code, debug logs etc.)
  • Self-reviewed and ran all changes on a local instance

Describe the reason for change

4 tests started to fail because one image from user.fm domain became unreacheable, so we had to disable them in #1706. In general it's dangerous to rely on 3rd party resources, so we had to switch to images hosted on our premises.

What alternative approaches were there?

Leave those tests disabled, but that's not a good way to go, especially for image tests, they are important.

What level of testing was included in the change?

  • e2e (codecept)
  • integration (cypress)
  • unit (jest)

@github-actions github-actions bot added the ci label Feb 23, 2024
@hlomzik hlomzik force-pushed the ci-leap-753/e2e-image-problems branch from 8526a80 to 9d0b9f9 Compare February 24, 2024 00:14
hlomzik added a commit that referenced this pull request Feb 25, 2024
hlomzik added a commit that referenced this pull request Feb 26, 2024
* fix: LEAP-740: Don't use destroy = null

There are external places that can call destroy unconditionally,
so having it uninitialized is dangerous and leads to app crash.

* Fail E2E test if Image is not loaded

* Revert destroy() behavior to original one

* Trigger CI

* Skip failing tests for now (until #1708)
MasherJames pushed a commit to HelloPareto/label-studio-frontend that referenced this pull request Feb 29, 2024
* fix: LEAP-740: Don't use destroy = null

There are external places that can call destroy unconditionally,
so having it uninitialized is dangerous and leads to app crash.

* Fail E2E test if Image is not loaded

* Revert destroy() behavior to original one

* Trigger CI

* Skip failing tests for now (until HumanSignal#1708)
- Calculate natural image size to calculate a real zoom on init (~1.05)
- Remove unnecessary waits, especially the one for 10(!) seconds
- Enable DEV-3377 FF to fix region shift on zoom
- Enable DEV-3793 FF to use relative coords for proper calculations
- Fix the way FF applied — they should be set before the `I.amOnPage()`
- Precision is increased to 2 (lower precisions result in rounding errors)
- Added comments to understand the flow of the test and exact calculations
Comment on lines +263 to +264
// ...relatively to displayed image size, which is canvas size when we open the page
Copy link
Contributor

Choose a reason for hiding this comment

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

really helpful comments here and below, nice

Copy link
Contributor

@bmartel bmartel left a comment

Choose a reason for hiding this comment

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

Really well commented. Thanks @hlomzik!

@hlomzik hlomzik merged commit 9d3e834 into master Mar 5, 2024
15 checks passed
@hlomzik hlomzik deleted the ci-leap-753/e2e-image-problems branch March 5, 2024 13:05
hlomzik added a commit to HumanSignal/label-studio that referenced this pull request Mar 26, 2024
hlomzik added a commit to HumanSignal/label-studio that referenced this pull request Mar 27, 2024
* feat: LEAP-219: Update LSF E2E with recent changes

- HumanSignal/label-studio-frontend#1192 for Magic Wand from @BradNeuberg
- HumanSignal/label-studio-frontend#1708 with external images problem
- HumanSignal/label-studio-frontend#1021 for maxUsages missed for some reason

* Fix newlines in all LSF E2E from CRLF to LF

* ci: Build frontend

Workflow run: https://github.com/HumanSignal/label-studio/actions/runs/8429166524

* Update web/libs/editor/tests/e2e/tests/ner-text.test.js

* ci: Build frontend

Workflow run: https://github.com/HumanSignal/label-studio/actions/runs/8429666577

---------

Co-authored-by: robot-ci-heartex <robot-ci-heartex@users.noreply.github.com>
Co-authored-by: bmartel <brandonmartel@gmail.com>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants