Minor fixes for tests #4686
Minor fixes for tests #4686
Conversation
This is consistent with the test immediately below.
|
I realised overnight the test immediately below the one I changed used a static atomic variable to get results out of the component - which looks much cleaner and simpler than what I wrote. I've updated the PR to match. I'll go looking for other tests that could do with updating. |
|
See also my comment #4685 (comment) which asks about another possible solution |
This should fix a handful of tests that didn't correctly do their test. In other places, comments should make it clearer *why* tests are how they are.
| } | ||
|
|
||
| #[test] | ||
| #[ignore] // Test doesn't do anything. |
There was a problem hiding this comment.
I might have misunderstood but I don't think this test generates any errors to bubble? I've marked it as ignore to flag it, but I'm open to changing or removing it.
| } | ||
| } | ||
|
|
||
| #[component] |
| fn nested_suspense_resolves_client() { | ||
| use Mutation::*; | ||
|
|
||
| async fn poll_three_times() { |
There was a problem hiding this comment.
This exactly duplicated a function at the top of the file so removed for DRYness
| fn create_without_cx() -> Signal<String> { | ||
| Signal::new("hello world".to_string()) | ||
| } | ||
|
|
There was a problem hiding this comment.
I wasn't sure if this level of checking was overkill, but it checks that the signals are working...
martin-hughes
changed the title
|
I looked through all of the Ready for review, happy to do any updates you want. |
I questioned in issue #4685 whether the tests could be fixed using a sync channel to move the assertion to the main function body.
This PR demonstrates that.
It's not intended to be merged just yet. Instead I wanted to see if this was an approach you'd support for the other tests, and if so I'll go looking for similar problems and update this PR.
Any comments welcome of course! Especially if you know of an easier way to achieve the same thing.
Just FYI, I haven't run the full set of tests from https://dioxuslabs.com/learn/0.6/contributing/ - I will do so if you get to the point of considering a merge.
Notes:
cargo testto see this)