Skip to content

Conversation

@kajarenc
Copy link
Collaborator

@kajarenc kajarenc commented Nov 17, 2022

📚 Context

These changes should hopefully fix test flakiness for the camera_input e2e test.
The reason, why flakiness was happening, is because Take photo is active even when the video stream from the browser is not ready yet.

Unfortunately, thereact-webcam library is not capable to provide us readiness event, the onUserMedia event is fired at the point when the browser had allowed to get video stream (for example when a user clicked Allow for the camera permission prompt), but actual video stream starts work after some period of time. So waiting 1 second before clicking Take photo button should be enough.

  • What kind of change does this PR introduce?

    • Bugfix
    • Feature
    • Refactoring
    • Other, please describe:

🧠 Description of Changes

  • Add bullet points summarizing your changes here

    • This is a breaking API change
    • This is a visible (user-facing) change

Revised:

Insert screenshot of your updated UI/code here

Current:

Insert screenshot of existing UI/code here

🧪 Testing Done

  • Screenshots included
  • Added/Updated unit tests
  • Added/Updated e2e tests

🌐 References

Does this depend on other work, documents, or tickets?

  • Issue: Closes #XXXX

Contribution License Agreement

By submitting this pull request you agree that all contributions to this project are made under the Apache 2.0 license.

@kajarenc kajarenc changed the title [WIP] Hopefully fix camera_input flaky test Fix (hopefully) camera_input flaky test Nov 17, 2022
@kajarenc kajarenc marked this pull request as ready for review November 17, 2022 18:49
@kajarenc kajarenc changed the title Fix (hopefully) camera_input flaky test Fix camera_input flaky test (hopefully) Nov 17, 2022
@mayagbarnes mayagbarnes self-requested a review November 17, 2022 19:11

// Add timeout until image is displayed
cy.get("img", { timeout: 10000 }).should("have.length.at.least", 2);
cy.get("img", { timeout: 1000 }).should("have.length.at.least", 2);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Instead of reducing this custom timeout, I would suggest removing it altogether - along with the comment on line above - since the default Cypress timeout is 4000 milliseconds anyways (doc)

cy.get("img").should("have.length.at.least", 2);

@kajarenc kajarenc added the security-assessment-completed Security assessment has been completed for PR label Nov 18, 2022
@mayagbarnes
Copy link
Collaborator

mayagbarnes commented Nov 18, 2022

Looks like the py_snowflake test (Snowpark integration test) is failing because your PR is from a fork and therefore doesn't have necessary secrets access (changes from this PR).
Good news it isn't blocking, but I'm working on a PR to fix anyway!

@mayagbarnes
Copy link
Collaborator

@kajarenc you should be good to integrate latest develop & merge 👍🏼

@kajarenc kajarenc merged commit 951bfaf into streamlit:develop Nov 24, 2022
tconkling added a commit to tconkling/streamlit that referenced this pull request Nov 28, 2022
* develop:
  Improve error message when widgets of different types have the same key (streamlit#5748)
  Add a new make target to only install build dependencies (streamlit#5755)
  Fix camera_input flaky test (hopefully) (streamlit#5714)
  Fix py_snowflake job for Nightly Release (streamlit#5751)
kmcgrady pushed a commit that referenced this pull request Nov 30, 2022
* Fix camera_input flaky test (hopefully)
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