-
Notifications
You must be signed in to change notification settings - Fork 6
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: fixed integration tests and selenium version #36
Conversation
8cf66d4
to
facba62
Compare
|
||
describe("Token Trade Proposals", () => { | ||
let daoAddress: string; | ||
let addresses: ITestAddresses; | ||
let addresses: any; |
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.
Why this change?
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.
@roienatan because interface is incorrect and it lead typescript checks issue. Proper way is just fix interface I gues...
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.
@roienatan trying to fix, but interface is deeply different. Code works properly, but we don't have correct interface for it
test/integration/tsconfig.json
Outdated
"moduleResolution": "node", | ||
"declaration": true, | ||
"declarationDir": "../dist/types", | ||
"removeComments": true, |
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.
this reduce bundle size?
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.
@orenyodfat hm... actually no. Bundle doesn't contain integration tests. I just copied this config from documentation. I can remove it. It is redundant and doesn't do anything useful
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.
@orenyodfat removed because of redundant
Is it normal that there is one check here? (instead of two checks) |
It is because of no branch exists at the same repo. I created PR from fork, because I had no access to this repo. So, just only PR checked, no branch. Actually, we should rather skip second check for all our requests, because one of them is redundant |
No description provided.