-
Notifications
You must be signed in to change notification settings - Fork 23
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
move crypto provider installer to be called once #1426
move crypto provider installer to be called once #1426
Conversation
I added the FFI function in zingo-mobile: |
i am proud of these sync tests. i understand that they have variable run time, but i do not understand why they would be ignored |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Whats our stance on expects and unwraps? Do we allow them in tests only?
@zancas @Oscar-Pepper
as you said, we cant have tests unignored that are guaranteed to slow down test run time, they will just get ignored some time in the near future anyway and cost a load of CI / local test developer time. this is what darkside is for, we have chain load framework. the only limitation I am aware of is transparent |
unwraps are test-only. expects can be acceptable in production with a message that eludes to why it should NEVER panic. |
In order to guarantee non-panic on an Please do NOT add |
I disagree. This is clear evidence that we do not have sufficient resources allocated to CI. We should not degrade our test quality because the free tier of github runners is too slow. That's the wrong direction. We need to invest in test infrastructure. |
Alright I added some non-blocking comments. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Given @juanky201271 's test, and caveats about upgrading infrastructure.. I approve.
this PR must be followed by an addition to zingo-mobile rust to also call this installer on app launch