-
Notifications
You must be signed in to change notification settings - Fork 1
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: test errors #78
base: main
Are you sure you want to change the base?
fix: test errors #78
Conversation
myz1237
commented
Nov 4, 2024
•
edited
Loading
edited
- Remove all tests to check if explorers are accessible. Reason: some fetch may fail every time when we run the test.
- Remove unavailable rps
|
expect(response.url).toBe(blockExplorerUrl) | ||
expect(response.ok).toBe(true) | ||
expect(isSameUrl(blockExplorerUrl, response.url)).toBeTruthy() | ||
expect(response.ok).toBeTruthy() |
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.
hmm what is ok
in that case? Because 'false'
is truthy, so do we really want this behaviour?
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.
- To be honest, I do not wanna this test cases for solana, evm and utxo, since the test really calls external service, which are unstable, that's the reason why we fail at most of time.
Any input @chybisov You added these tests, is it fine to remove it and just check if the url is legal or not?
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.
@chybisov Do we wanna remove it? Otherwise the test does not work.
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.
Solana explorers should pass tests fine, there were some issues with Bitcoin ones. Would you like to remove these checks completely?
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.
yes, I wanna remove them completely, and just ensure the rpc and explorer array are not empty.
Normally, we add multiple rpcs, some of them may be invalid, or we are blocked. Same to explorer, the explorer could refuse our request, which are unstable.
what do you think of it?
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.
I have removed all tests about the block explorer links
Any updates @myz1237 ? |
@@ -35,7 +35,6 @@ export const supportedEVMChains: EVMChain[] = [ | |||
'https://ethereum-rpc.publicnode.com', | |||
'https://eth.drpc.org', | |||
'https://eth.public-rpc.com', | |||
'https://mainnet.infura.io/v3/9aa3d95b3bc440fa88ea12eaa4456161', |
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.
key is invalid now
@@ -794,7 +793,6 @@ export const supportedEVMChains: EVMChain[] = [ | |||
'https://mantle-rpc.publicnode.com', | |||
'https://mantle.drpc.org', | |||
'https://mantle.public-rpc.com', | |||
'https://rpc.ankr.com/mantle', |
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.
unavaialble
If this pr is merged, please set the merge policy: only allowed to merge if all actions succeed |