-
Notifications
You must be signed in to change notification settings - Fork 99
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
feat: new automation scope for tests on Contracts API + Tokens API #170
feat: new automation scope for tests on Contracts API + Tokens API #170
Conversation
API E2E Test Results206 tests 206 ✅ 20s ⏱️ Results for commit 98f183d. ♻️ This comment has been updated with latest results. |
Visit the preview URL for this PR (updated for commit 98f183d):
(expires Fri, 12 Apr 2024 12:19:36 GMT) 🔥 via Firebase Hosting GitHub Action 🌎 Sign: e508f9012944951194447cb8885950b451a24403 |
Unit Test Results 4 files 263 suites 11m 55s ⏱️ Results for commit 98f183d. ♻️ This comment has been updated with latest results. |
@pcheremu can you please review the PR? |
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.
@olehbairak please, apply changes based on comments
@pcheremu thank you for the feedback! |
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.
@abilevych all essential changes were applied. I'm still thinking about helper.performGETrequest(apiRoute)
. From the one hand we can have an agreement that if we use just one variable it will mean that the network will be local, from the other hand, as for me, it will be more clear to have two arguments helper.performGETrequest(apiRoute, network)
to transfer there more explicitly. Both options are make sense, please let us know what would you prefer in this case?
@@ -61,8 +61,14 @@ export class Helper { | |||
return new Promise((resolve) => setTimeout(resolve, ms)); | |||
} | |||
|
|||
async performGETrequest(apiRoute: string) { | |||
return request(environment.blockExplorerAPI).get(apiRoute); | |||
async performGETrequest(apiRoute: string, network = "local") { |
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.
Also, I assume the name "environment" is a better option for a parameter than the "network" is.
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.
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.
fixed
@@ -22,6 +22,59 @@ describe("API module: Contract", () => { | |||
await playbook.deployMultiCallContracts(); | |||
}); | |||
|
|||
//id1851 |
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'd mark here by tag\comment that tests or suite related to the Sepolia network (or environment)
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.
Added
await helper.retryTestAction(async () => { | ||
apiRoute = `/api?module=contract&action=checkverifystatus&guid=3177`; | ||
response = await helper.performGETrequest(apiRoute, "sepolia"); | ||
console.log(response.body); |
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.
Is that necessary to have it here?
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.
Removed
@pcheremu @abilevych I fixed comments. |
…sapi-accounts-api-automate-test-cases-for-contracts-api
…rak-qa-653-tokensapi-accounts-api-automate-test-cases-for-contracts-api"" This reverts commit 7fa1b56.
…sapi-accounts-api-automate-test-cases-for-contracts-api
…sapi-accounts-api-automate-test-cases-for-contracts-api
…sapi-accounts-api-automate-test-cases-for-contracts-api
@abilevych conflicts were resolved. |
|
@olehbairak I messed up a bit with git (closed the PR by mistake). please re-start all those checks if it's needed and once it's done you can merge it by yourself. Sorry for some inconvenience |
@pcheremu can you please resolve the request change - it's currently blocking from merge. |
What ❔
Implemented automation test coverage for next test cases:
https://linear.app/matterlabs/issue/QA-653/%5Btokensapi-contracts-api%5D-automate-test-cases-for-contracts-api-for-be
Why ❔
To check this functionality in PR automation integration tests.
Checklist