Skip to content

Conversation

@tconkling
Copy link
Contributor

Lots of drive-by cleanup on all our widget tests, in preparation for the incoming "clear-on-submit" forms PR.

  • Remove redundant "should do X" from test descriptions (it("renders a label"), instead of it("should render a label"))
  • Fix WidgetStateManager mocking
  • Move props and wrapper instances outside of global scope, and into individual test scopes
  • Don't mock all of WidgetStateManager. Instead, use jest.spyOn on the single WidgetStateManager function each widget uses
  • Add a handful of tests for widgets with low coverage

@tconkling tconkling requested a review from a team May 18, 2021 00:13
Copy link
Collaborator

@kantuni kantuni left a comment

Choose a reason for hiding this comment

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

👍

@tconkling tconkling merged commit a5076d9 into streamlit:develop May 19, 2021
@tconkling tconkling deleted the tim/WidgetTestCleanup branch May 19, 2021 17:03
tconkling added a commit to tconkling/streamlit that referenced this pull request May 19, 2021
* develop:
  Widget test cleanup (streamlit#3286)
  pin click to < 8.0 (streamlit#3256)
  Up version to 0.82.0
  Update change log
tconkling added a commit that referenced this pull request May 24, 2021
Implements `clear_on_submit`, a new optional parameter for st.form. When a form has this flag, it means that the frontend "resets" all the form's widgets when the form is submitted.

CustomComponents are _not_ currently affected by this flag (this is by design - properly handling this would require a CustomComponents API update, and all widget-like components in the wild would need updating. So we're punting on it for now).

- `FormsManager` is now folded into `WidgetStateManager`.
- When a form is submitted, `WidgetStateManager` emits a signal. All widgets in the form respond to this signal by resetting their values locally and committing those values back to the WidgetStateManager.
- This resetting only happens on the frontend - the server doesn't know that values have been reset (this is by design).
- `FormClearHelper` is a utility class that implements a `useEffect`-like pattern for subscribing to the `formCleared` events. (None of our widgets are currently implemented using React hooks, so this is a workaround for easily and correctly composing that behavior into each widget.)
- Each widget has a new test for `clearOnSubmit` behavior.
- `st.form_submit_button` now takes an optional "help" parameter.

(Contains changes from #3286, which should be reviewed first)
tconkling added a commit to tconkling/streamlit that referenced this pull request May 25, 2021
# By Ken McGrady (19) and others
# Via GitHub (9) and others
* develop: (96 commits)
  st.form: clear_on_submit (streamlit#3287)
  Init test variables in beforeEach
  Init test variables in beforeEach
  Add database tutorials to docs (streamlit#3272)
  update contact email to support (streamlit#3282)
  Better typing
  Remove dummy script
  Add testing and some comments
  Fix the bug, don't love the solution
  Improve test name
  Add script for testing
  Fix test after merge
  Make sure selected options aren't visible in the dropdown
  Widget test cleanup (streamlit#3286)
  Add test for the fuzzy filter
  Import filter function from another component
  Remove tag from PR template (streamlit#3284)
  Add test req version bound for TF to fix tests (streamlit#3266)
  pin click to < 8.0 (streamlit#3256)
  Up version to 0.82.0
  ...

# Conflicts:
#	frontend/src/components/shared/Dropdown/Selectbox.test.tsx
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.

2 participants