-
Notifications
You must be signed in to change notification settings - Fork 16
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
Modify tests so that account authorization tests will work for happy path #154
Modify tests so that account authorization tests will work for happy path #154
Conversation
…path mod account authorization tests so they will work with develop branch of contracts and fio
swallow error on linkauth, if we use same permission repeatedly this is a 500 error but the auth test should stilll pass.
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.
Please, check comments.
Other thing do we need to make the same updates into testnet
test file?
tests/index.spec.js
Outdated
const account1 = FIOSDK.accountHash(publicKey).accountnm; | ||
const account2 = FIOSDK.accountHash(publicKey2).accountnm; | ||
|
||
describe.only('Test addaddress on account with permissions', () => { |
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 guess .only
should be removed.
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, will do...
tests/index.spec.js
Outdated
parent: 'active', | ||
auth: { |
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.
We have authorization_object
for auth
. Let's left and use it or remove it and left updates. Both choices works for me.
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 will use it...
@@ -869,7 +910,9 @@ describe('Test addaddress on account with permissions', () => { | |||
expect(result.block_num).to.be.a('number'); | |||
expect(result.transaction_id).to.be.a('string'); | |||
} catch (e) { | |||
console.log(e); | |||
//the error we get here is due to using the same account every time we run the test, | |||
//we get an error "same as previous" from linkauth, this is ok! |
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.
Could we handle error message somehow? It would be awesome if we could handle that specific error and return error if it's something else.
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.
for this one test, if the linkauth fails for another reason, then the next test will fail....so the test will fail.
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.
Oh, I see. That makes sense.
code review findings.
mod account authorization tests so they will work with develop branch of contracts and fio