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

Tests for checking image change upon change in options #1490

Merged
merged 13 commits into from
Jan 17, 2020

Conversation

rishabhshuklax
Copy link
Member

@rishabhshuklax rishabhshuklax commented Jan 13, 2020

Fixes #1447

Make sure these boxes are checked before your pull request (PR) is ready to be reviewed and merged. Thanks!

  • tests pass -- look for a green checkbox ✔️ a few minutes after opening your PR -- or run tests locally with npm test
  • code is in uniquely-named feature branch and has no merge conflicts
  • PR is descriptively titled
  • ask @publiclab/is-reviewers for help, in a comment below
  • Insert-step functionality is working correct as expected.

Tests for the following modules have been added in this PR.

  • Blur
  • Brightness
  • Channel
  • Color Temperature
  • Colorbar
  • Colormap
  • Contrast
  • Convolution
  • Dither
  • Dynamic
  • Exposure
  • Flip Image
  • Gamma Correction
  • Grid Overlay
  • NDVI
  • Noise Reduction
  • Replace Color
  • Rotate
  • Saturation
  • Threshold
  • Tint
  • White Balance

@rishabhshuklax
Copy link
Member Author

@keshav234156 @jywarren @harshkhandeparkar What are your thoughts on this?
Should we test the options change this way or do you have any other plan in mind?

@codecov
Copy link

codecov bot commented Jan 13, 2020

Codecov Report

Merging #1490 into main will not change coverage.
The diff coverage is 91.66%.

Impacted file tree graph

@@           Coverage Diff           @@
##             main    #1490   +/-   ##
=======================================
  Coverage   66.98%   66.98%           
=======================================
  Files         128      128           
  Lines        2626     2626           
  Branches      424      424           
=======================================
  Hits         1759     1759           
  Misses        867      867
Impacted Files Coverage Δ
examples/lib/defaultHtmlStepUi.js 10.98% <0%> (ø) ⬆️
src/modules/Blend/Module.js 88.46% <100%> (ø) ⬆️
src/modules/Dynamic/Module.js 78.94% <100%> (ø) ⬆️

@rishabhshuklax
Copy link
Member Author

@publiclab/is-reviewers


/**
* @method optionsChangeTest
* @description a common test for modules
Copy link
Member

Choose a reason for hiding this comment

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

Please capitalize the first letter and add a full stop at the end of a comment.

Copy link
Member Author

Choose a reason for hiding this comment

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

Done!

test/core/templates/options-test.js Show resolved Hide resolved
@rishabhshuklax
Copy link
Member Author

@harshkhandeparkar Do you think this is the best way to test this? Or should I look for an alternative?

@harshkhandeparkar
Copy link
Member

I guess it is good for now. I don't see any downside to this as of now. (But if we change the modules in the future, it may be a problem)

@rishabhshuklax
Copy link
Member Author

If we change the modules in future the test will fail and corresponding benchark/options can be changed easily, i don't think that would be a problem.

@harshkhandeparkar
Copy link
Member

Great. Please make the requested changes.

@rishabhshuklax rishabhshuklax changed the title [WIP] Adding tests for checking image change upon change in options Tests for checking image change upon change in options Jan 17, 2020
@jywarren jywarren merged commit 89fb358 into publiclab:main Jan 17, 2020
@jywarren
Copy link
Member

Wow, this is really super. Great work, @blurry-x-face !!!

Thanks a ton!

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.

Adding tests for checking image change upon change in options
3 participants