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: add tests to model settings's Slider Input #3615

Merged
merged 4 commits into from
Sep 10, 2024

Conversation

louis-jan
Copy link
Contributor

@louis-jan louis-jan commented Sep 10, 2024

Describe Your Changes

This PR is a follow-up fix for #3609, where users could get stuck when inputting a value to the model settings input component.

In this PR, it covers the case where users could get stuck in a broken state after inputting an invalid settings value and then force quitting the app immediately. This behavior is hard to reproduce but possible.

Rework web test configurations to resolve the collision between Jest Babel and Next Babel.

Tests have been added to cover such cases.

Context

We've received a broken app report while testing a feature build; after investigating, we found the root cause and fixed it. However, tests have not been added in this PR due to the incomplete setup.

This PR is to add tests but it turned out that there are missing cases to handle.

Related PRs

Self Checklist

  • Added relevant comments, esp in complex areas
  • Updated docs (for bug fixes / features)
  • Created issues for follow-up changes or refactoring needed

@github-actions github-actions bot added the type: chore Maintenance, integration, packaging related label Sep 10, 2024
@louis-jan
Copy link
Contributor Author

cc @dan-homebrew, regarding the test covers you mentioned.

@urmauur
Copy link
Member

urmauur commented Sep 10, 2024

LGTU -> looks good to us 💚

Copy link
Contributor

github-actions bot commented Sep 10, 2024

Barecheck - Code coverage report

Total: 54.09%

Your code coverage diff: 0.00% ▴

✅ All code changes are covered

Copy link
Contributor

@dan-homebrew dan-homebrew left a comment

Choose a reason for hiding this comment

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

@louis-jan Can I verify the nature of this bug:

  • Is it UI related, or related to out-of-bounds error on params?
  • The tests seem to be UI-related
  • Do we need corresponding out-of-bounds tests at the API level?

@louis-jan
Copy link
Contributor Author

@louis-jan Can I verify the nature of this bug:

  • Is it UI related, or related to out-of-bounds error on params?
  • The tests seem to be UI-related
  • Do we need corresponding out-of-bounds tests at the API level?

Yes @dan-homebrew there is 1 more PR to add cover persistence layer. E.g. where all data are gathered and persisted. And that one is even more important.

@louis-jan
Copy link
Contributor Author

louis-jan commented Sep 10, 2024

For the context,
I separated the PR tests, this small one is to fix the Nightly Build CI break for now. (138-139)

cc @hiento09

@louis-jan louis-jan force-pushed the chore/add-tests-to-model-settings-input-component branch from e17c59b to 8047ed6 Compare September 10, 2024 08:49
@louis-jan louis-jan merged commit 2d05134 into dev Sep 10, 2024
9 checks passed
@louis-jan louis-jan deleted the chore/add-tests-to-model-settings-input-component branch September 10, 2024 09:36
@github-actions github-actions bot added this to the v.0.6.0 milestone Sep 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: chore Maintenance, integration, packaging related
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants