Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(ci): "needless borrow" error and example never exiting #2392

Merged
merged 4 commits into from
Mar 1, 2024

Conversation

paul-hansen
Copy link
Contributor

@paul-hansen paul-hansen commented Mar 1, 2024

options is already a reference so it was passing a double reference. Looks like this was inadvertently added in the last commit in PR #2373 while other CI failures were happening so it was missed.

The error_boundary example has been leaving the server running and eventually failing due to timing out. This PR makes the start task run in parallel with the playwright task instead of waiting forever on the server start task to exit it uses a dumb two minute sleep timer to ensure the server has compiled and started. This could probably be improved but might risk being too complex for bash scripts so I left it for now.

@gbj
Copy link
Collaborator

gbj commented Mar 1, 2024

Thanks for both of these commits: truly heroic to investigate and fix weird CI behavior 😄

Will let CI run (hopefully for less than 6 hours :-p) and merge if it's good.

@paul-hansen paul-hansen force-pushed the fix-ci branch 2 times, most recently from 4752a4b to 3cab09e Compare March 1, 2024 15:38
@paul-hansen
Copy link
Contributor Author

paul-hansen commented Mar 1, 2024

Thanks for Leptos! Happy to contribute where I can.

Unfortunately it didn't work to kill the example 😞
Looks like that example is the only one to use playwright-trunk-test.toml, I'll look at that some, if I don't get it figured out I'll pull that commit from the PR so we can at least merge the other fix.

It took longer than I thought in Github and barely worked, giving it a
bit more of a buffer.
@paul-hansen
Copy link
Contributor Author

New fix is working, updated the description in the first post with some details. Looks like the integration/axum CI is failing now but that shouldn't be related to this PR as I only changed the cargo-make files in the examples which isn't used there.

@paul-hansen
Copy link
Contributor Author

paul-hansen commented Mar 1, 2024

Interestingly one of the lines the axum integration is failing on isn't even in the branch for this PR:
https://github.com/leptos-rs/leptos/actions/runs/8117600089/job/22190537155?pr=2392#step:17:4323

error[E0425]: cannot find function `expect_context` in this scope
 --> integrations/axum/src/lib.rs:94:16
  |
7 |     let opts = expect_context::<leptos_axum::ResponseOptions>();
  |                ^^^^^^^^^^^^^^ not found in this scope
  |
help: consider importing this function
  |
2 + use leptos::expect_context;
  |

That line was added in this commit:
f70ebc1

@gbj
Copy link
Collaborator

gbj commented Mar 1, 2024

Thanks!

@gbj gbj merged commit de25658 into leptos-rs:main Mar 1, 2024
49 of 60 checks passed
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