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

Integration Test for RPC nodes #2984

Open
wants to merge 8 commits into
base: master
Choose a base branch
from

Conversation

keval-finimble
Copy link
Contributor

No description provided.

@github-actions
Copy link

Pull reviewers stats

Stats of the last 30 days for alpha-wallet-android:

User Total reviews Time to review Total comments
seabornlee
🥇
16
▀▀▀▀
3h 28m
12
▀▀▀▀▀▀
JamesSmartCell
🥈
14
▀▀▀▀
18h 56m
▀▀▀▀
7
▀▀▀▀
justindg
🥉
8
▀▀
20h 33m
▀▀▀▀▀
1

@keval-finimble keval-finimble changed the title - Integration Test for RPC nodes Integration Test for RPC nodes Nov 29, 2022
@codecov
Copy link

codecov bot commented Nov 29, 2022

Codecov Report

Patch coverage: 5.49% and project coverage change: -0.02 ⚠️

Comparison is base (bca67f6) 7.58% compared to head (99f6565) 7.56%.

❗ Current head 99f6565 differs from pull request most recent head 7d93694. Consider uploading reports for the commit 7d93694 to get more accurate results

Additional details and impacted files
@@             Coverage Diff             @@
##             master   #2984      +/-   ##
===========================================
- Coverage      7.58%   7.56%   -0.02%     
  Complexity      906     906              
===========================================
  Files           658     658              
  Lines         44857   44876      +19     
  Branches       4517    4524       +7     
===========================================
- Hits           3402    3396       -6     
- Misses        41165   41190      +25     
  Partials        290     290              
Impacted Files Coverage Δ
app/src/main/java/com/alphawallet/app/C.java 57.14% <ø> (ø)
...ain/java/com/alphawallet/app/entity/EventSync.java 0.00% <0.00%> (ø)
...wallet/app/interact/FetchTransactionsInteract.java 22.22% <0.00%> (-1.31%) ⬇️
...hawallet/app/repository/TransactionRepository.java 7.86% <0.00%> (-0.19%) ⬇️
...n/java/com/alphawallet/app/service/GasService.java 7.82% <0.00%> (+0.17%) ⬆️
...ava/com/alphawallet/app/service/TickerService.java 19.23% <ø> (+0.66%) ⬆️
...m/alphawallet/app/service/TransactionsService.java 3.82% <0.00%> (-0.05%) ⬇️
.../java/com/alphawallet/app/ui/ActivityFragment.java 0.00% <0.00%> (ø)
...com/alphawallet/app/ui/NFTAssetDetailActivity.java 0.00% <0.00%> (ø)
...ain/java/com/alphawallet/app/ui/TokenActivity.java 0.00% <0.00%> (ø)
... and 6 more

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

Copy link
Member

@JamesSmartCell JamesSmartCell left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good start - can you also add to the test to verify that each network doesn't display a :( face. I am not sure the best way to do this; a good way could be to sense the "IC" value in the environment variables (I think it is "IC") and if sensed then if any network comes up with a :( switch on some text (or symbol) at the top of the page. The integration test would sense that text.

To do this correctly you'd also need to display some different text (or symbol) once all networks have completed their check.

This way you can timeout, complete if the 'all network success' symbol/text comes up and mark fail if the 'one network has failed' symbol/text becomes visible.

This symbol/text should only appear if the app senses that it is running an integration test. I think the "IC" symbol will appear in the environment vars if it is (check the script).

@seabornlee
Copy link
Contributor

Good start - can you also add to the test to verify that each network doesn't display a :( face. I am not sure the best way to do this; a good way could be to sense the "IC" value in the environment variables (I think it is "IC") and if sensed then if any network comes up with a :( switch on some text (or symbol) at the top of the page. The integration test would sense that text.

To do this correctly you'd also need to display some different text (or symbol) once all networks have completed their check.

This way you can timeout, complete if the 'all network success' symbol/text comes up and mark fail if the 'one network has failed' symbol/text becomes visible.

This symbol/text should only appear if the app senses that it is running an integration test. I think the "IC" symbol will appear in the environment vars if it is (check the script).

Do you mean "CI" value in environment variables?

@JamesSmartCell
Copy link
Member

Good start - can you also add to the test to verify that each network doesn't display a :( face. I am not sure the best way to do this; a good way could be to sense the "IC" value in the environment variables (I think it is "IC") and if sensed then if any network comes up with a :( switch on some text (or symbol) at the top of the page. The integration test would sense that text.
To do this correctly you'd also need to display some different text (or symbol) once all networks have completed their check.
This way you can timeout, complete if the 'all network success' symbol/text comes up and mark fail if the 'one network has failed' symbol/text becomes visible.
This symbol/text should only appear if the app senses that it is running an integration test. I think the "IC" symbol will appear in the environment vars if it is (check the script).

Do you mean "CI" value in environment variables?

That's exactly what I mean :)

seabornlee and others added 3 commits May 6, 2023 22:15
* Update transfer fetch

* Fix test and formatting
@seabornlee
Copy link
Contributor

Hi @JamesSmartCell, the test is currently failing and I’m unsure whether it’s due to issues with the test code or if some of the networks are not responding. Could you please verify the network status on the test device?

@JamesSmartCell
Copy link
Member

Hi @JamesSmartCell, the test is currently failing and I’m unsure whether it’s due to issues with the test code or if some of the networks are not responding. Could you please verify the network status on the test device?

Yes! Artis has been switched off (we checked with the Artis team), so it needs to be removed. Then the test should work. I can see it failing instantly because Artis isn't responding.

@JamesSmartCell
Copy link
Member

Hi @JamesSmartCell, the test is currently failing and I’m unsure whether it’s due to issues with the test code or if some of the networks are not responding. Could you please verify the network status on the test device?

Yes! Artis has been switched off (we checked with the Artis team), so it needs to be removed. Then the test should work. I can see it failing instantly because Artis isn't responding.

Hi @JamesSmartCell, the test is currently failing and I’m unsure whether it’s due to issues with the test code or if some of the networks are not responding. Could you please verify the network status on the test device?

Hi @JamesSmartCell, the test is currently failing and I’m unsure whether it’s due to issues with the test code or if some of the networks are not responding. Could you please verify the network status on the test device?

Hi @JamesSmartCell, the test is currently failing and I’m unsure whether it’s due to issues with the test code or if some of the networks are not responding. Could you please verify the network status on the test device?

I removed the ARTIS chain

Now it's giving this error:

com.alphawallet.app.RPCNodesTest > should_select_network[Pixel - 10] FAILED 
	androidx.test.espresso.base.DefaultFailureHandler$AssertionFailedWithCauseError: 'not (view is an instance of android.view.ViewGroup and has descendant matching (view.getId() is <2131362276> and has background resource with id 2131230998))' doesn't match the selected view.
	Expected: not (view is an instance of android.view.ViewGroup and has descendant matching (view.getId() is <2131362276/io.stormbird.wallet:id/image_status> and has background resource with id 

It might be an idea to wait for 20 seconds or so for all the nodes to return the ping; initially there may not be any status icons visible.

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.

3 participants