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

test: Acceptance tests seems inconsistent #430

Merged
merged 1 commit into from
Aug 9, 2022

Conversation

georgi-l95
Copy link
Collaborator

@georgi-l95 georgi-l95 commented Aug 8, 2022

Description:
Fixing two tests with these changes. Another one was fixed here hashgraph/hedera-sdk-js#1224.

Logic behind fix for test: should call eth_feeHistory with updated fees
In the before hook, we are feeding default fees to the mirror node. So that we know how many blocks we should take and where to expect those fees. But if there is discrepancy in the blocks, the first element in this array is taken from the consensus node, not from the mirror node. This is the reason sometimes this value in the first index is wrong.

Logic behind fix for test: should call eth_feeHistory with newest block > latest
By the time we're requesting the last block, it's already ahead by one. So we were excepting it to be 10 ahead, but it's actually 11 ahead on rare occasions.

Related issue(s):

Fixes #381

Notes for reviewer:

  131 passing (4m)

[2022-08-08 12:44:04.670 +0000] INFO (rpc-acceptance-test/44539 on Georgis-MacBook-Pro-3.local): Acceptance Tests spent 532.31120121 ℏ

Checklist

  • Documented (Code comments, README, etc.)
  • Tested (unit, integration, etc.)

@georgi-l95 georgi-l95 linked an issue Aug 8, 2022 that may be closed by this pull request
@Nana-EC Nana-EC added enhancement New feature or request limechain P2 labels Aug 8, 2022
@Nana-EC Nana-EC added this to the 0.6.0 milestone Aug 8, 2022
Copy link
Collaborator

@Nana-EC Nana-EC left a comment

Choose a reason for hiding this comment

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

Looking good.
Please add a logic comment to clarify the optimization

packages/server/tests/acceptance/rpc.spec.ts Show resolved Hide resolved
@codecov-commenter
Copy link

Codecov Report

Merging #430 (7e8f111) into main (c91245f) will increase coverage by 0.09%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##             main     #430      +/-   ##
==========================================
+ Coverage   73.55%   73.65%   +0.09%     
==========================================
  Files          10       10              
  Lines         832      835       +3     
  Branches      136      137       +1     
==========================================
+ Hits          612      615       +3     
  Misses        172      172              
  Partials       48       48              
Impacted Files Coverage Δ
packages/relay/src/lib/eth.ts 81.25% <0.00%> (+0.14%) ⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@georgi-l95 georgi-l95 requested a review from Nana-EC August 9, 2022 07:10
Nana-EC
Nana-EC previously approved these changes Aug 9, 2022
Copy link
Collaborator

@Nana-EC Nana-EC left a comment

Choose a reason for hiding this comment

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

LG.
Minor spacing and spelling mistake

packages/server/tests/acceptance/rpc.spec.ts Outdated Show resolved Hide resolved
@georgi-l95 georgi-l95 force-pushed the 381-inconsistent-acceptancetest branch from 00a10db to 78832c2 Compare August 9, 2022 15:14
@sonarcloud
Copy link

sonarcloud bot commented Aug 9, 2022

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

Copy link
Collaborator

@Nana-EC Nana-EC left a comment

Choose a reason for hiding this comment

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

LGTM

@georgi-l95 georgi-l95 merged commit 6674394 into main Aug 9, 2022
@georgi-l95 georgi-l95 deleted the 381-inconsistent-acceptancetest branch August 9, 2022 20:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request limechain P2
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

acceptancetest check seems inconsistent
3 participants