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 autosaving of gamma correction setting #9124

Open
wants to merge 9 commits into
base: develop
Choose a base branch
from

Conversation

archibald1418
Copy link
Contributor

@archibald1418 archibald1418 commented Feb 19, 2025

Update existing local storage test to include gamma setting checks.

Motivation and context

This is a test for storing gamma correction value alongside other settings inside localStorage.
Thanks to #9032, gamma is treated separately from other color settings. Unlike brightness, contrast and saturation settings, which change style attributes on #cvat_canvas_background, gamma is stored inside localStorage.clientSettings.imageFilter entity. This was the main motivation of updating the case_68_saving_settings_local_storage.js instead of writing a new testcase or updating the color settings test.

How has this been tested?

The main flow of the test stayed the same, but with couple of changes. Validation and setup parts were extended to include gamma.

Before the changes the test looked like this:

  1. Open job
  2. Setup workspace settings (check the boxes). Make sure that localStorage is set
  3. Reload
  4. Validate settings (should be checked)
  5. Repeat step 1 (this time uncheck the boxes)
  6. Reload
  7. Repeat step 3 (should be unchecked)

Now, gamma is set up before step 1 and validated after first reload. Also, color settings are reset after the gamma check to keep integrity between tests

Checklist

  • I submit my changes into the develop branch
  • I have created a changelog fragment
  • I have updated the documentation accordingly
  • I have added tests to cover my changes
  • I have linked related issues (see GitHub docs)

License

  • I submit my code changes under the same MIT License that covers the project.
    Feel free to contact the maintainers if that's a concern.

@codecov-commenter
Copy link

codecov-commenter commented Feb 20, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 73.83%. Comparing base (558a861) to head (2c8ebdf).
Report is 3 commits behind head on develop.

Additional details and impacted files
@@           Coverage Diff            @@
##           develop    #9124   +/-   ##
========================================
  Coverage    73.83%   73.83%           
========================================
  Files          431      431           
  Lines        44807    44804    -3     
  Branches      3892     3892           
========================================
- Hits         33084    33082    -2     
+ Misses       11723    11722    -1     
Components Coverage Δ
cvat-ui 77.53% <ø> (ø)
cvat-server 70.79% <ø> (+<0.01%) ⬆️

@@ -3,12 +3,33 @@
//
// SPDX-License-Identifier: MIT

/// <reference types="cypress" />
/// / <reference types="../../support/index.d.ts" />
Copy link
Contributor

Choose a reason for hiding this comment

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

This type file is not implemented yet

Copy link
Contributor Author

Choose a reason for hiding this comment

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

removed

@@ -4,6 +4,8 @@

/// <reference types="cypress" />

/* eslint security/detect-non-literal-fs-filename: 0 */
Copy link
Contributor

Choose a reason for hiding this comment

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

Will /* eslint-disable security/detect-non-literal-fs-filename */ work?
Looks a bit cleaner than : 0

Copy link
Contributor Author

@archibald1418 archibald1418 Feb 21, 2025

Choose a reason for hiding this comment

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

Yes, that works. Applied

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.

3 participants