Skip to content

Conversation

@tconkling
Copy link
Contributor

Our new cache function "widget replay" functionality relies on a ScriptRunContext existing: without a ScriptRunContext, no values will be written to the cached function.

Some of our caching tests don't create a ScriptRunContext. Running these tests in isolation (outside of the full test suite) currently results in test failures due to a missing context. (When run as part of the full test suite, some other ScriptRunContext-creating test will run first, which is why they don't fail in that circumstance.)

The new widget replay functionality relies on ScriptRunContext; running these tests in isolation (outside of the full test suite) currently results in test failures due to a missing context. (When run as part of the full test suite, some other ScriptRunContext-creating test will run first, which is why they don't fail in that circumstance.)
@tconkling tconkling added the security-assessment-completed Security assessment has been completed for PR label Nov 19, 2022
@tconkling tconkling requested a review from kajarenc November 21, 2022 19:49
class MemoTest(unittest.TestCase):
def setUp(self) -> None:
# Caching functions rely on an active script run ctx
add_script_run_ctx(threading.current_thread(), create_mock_script_run_ctx())
Copy link
Collaborator

Choose a reason for hiding this comment

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

Should we call super().setUp() since we call super().tearDown() in tear down?

@kajarenc
Copy link
Collaborator

LGTM 👍

Snowflake credentials test CI check is not blocking, we are good to merge PR!

@tconkling tconkling merged commit 6d4d1b3 into streamlit:develop Nov 21, 2022
@tconkling tconkling deleted the tim/FixSingletonAndMemoTests branch November 21, 2022 21:33
tconkling added a commit to tconkling/streamlit that referenced this pull request Nov 21, 2022
# By Abhik Lodh (1) and others
# Via GitHub
* develop:
  memo_test/singleton_test: ensure ScriptRunCtx exists (streamlit#5739)
  Update loadApp command timeout (streamlit#5731)
  Allow emojis with variant selectors to be validated (streamlit#5583)

# Conflicts:
#	lib/tests/streamlit/runtime/caching/cache_data_api_test.py
#	lib/tests/streamlit/runtime/caching/cache_resource_api_test.py
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

security-assessment-completed Security assessment has been completed for PR

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants