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

workflow: split running tests required to be on the main thread #134

Closed
wants to merge 1 commit into from

Conversation

eugenesvk
Copy link
Contributor

@eugenesvk eugenesvk commented Dec 9, 2024

into a separate workflow item with a custom argument so that the custom harness knows to use the main thread

Should hopefully fix the #128 test issues

…a separate workflow item

with a custom argument (also requires a custom harness supporting main threads)
@Byron
Copy link
Owner

Byron commented Dec 9, 2024

Let's keep this as part of #128 - that's what it needs to run with to see if it has the desired effect. Please note that the osakit tests also have to bail (or fail) if they are not run on the main thread.

@Byron Byron closed this Dec 9, 2024
@eugenesvk
Copy link
Contributor Author

eugenesvk commented Dec 9, 2024

Keeping it as part of 128 won't allow testing that pr since it will use the existing wrong workflow
I haven't errored on non-main thread just because it also sometimes works without the main thread

@Byron
Copy link
Owner

Byron commented Dec 9, 2024

Keeping it as part of 128 won't allow testing that pr since it will use the existing wrong workflow

I don't understand why that would be. If you know of some restriction this repo might have, I am happy to lift it.
For all I know, PRs can change the workflow and observe the changes as well. Otherwise there would be no way to test these.

@eugenesvk
Copy link
Contributor Author

Oh, seems your repo is already setup to work, you're right, guess the last time I've tried there was some issue that led to test failure

@eugenesvk eugenesvk deleted the fr-workflow-main-thread branch December 9, 2024 15:29
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