Skip to content

Conversation

@anotherjesse
Copy link
Contributor

@anotherjesse anotherjesse commented Mar 5, 2025

this builds upon #516 - adding more testing

@anotherjesse
Copy link
Contributor Author

@jakedahn @bfollington opinions on changes in jumble to support easier testing?

@anotherjesse
Copy link
Contributor Author

Questions:

  1. I had to sleep 5 seconds on CI vs no sleep locally.

rather than having "sleep" I wonder if we can have a more accurate "wait for green sync status"

  1. perhaps we should validate that we can see the changed value in the cli at the end

  2. aria-label-current-foo? what is the proper aria for the charm title/content

@anotherjesse
Copy link
Contributor Author

anotherjesse commented Mar 5, 2025

fixed:

  1. failing tests leave chrome open!
  2. re-running tests fail due to assumptions about data being empty at first - use a unique space/replica for each run

@anotherjesse
Copy link
Contributor Author

I also want to add running common-cli to validate the cells propagated out of the web context to common memory and back to cli

Copy link
Collaborator

@jsantell jsantell left a comment

Choose a reason for hiding this comment

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

Looking great, thank you! Some comments on alternatives and a potential race condition

@jsantell
Copy link
Collaborator

jsantell commented Mar 5, 2025

Got rid of most sleeps, but seeing this error (sometimes) when clicking the button inbetween charm renders: https://jsr.io/@astral/astral/0.5.2/src/element_handle.ts#L192

TBD if waiting a bit helps, still seems ultimately intermittent

@jsantell jsantell merged commit 1e4cddd into main Mar 5, 2025
6 checks passed
@jsantell jsantell deleted the more-smoke branch March 5, 2025 17:35
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.

3 participants