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

Create CoinFabrik_On_Ink_Integration_Tests_2.md #1980

Merged
merged 3 commits into from
Oct 23, 2023
Merged

Create CoinFabrik_On_Ink_Integration_Tests_2.md #1980

merged 3 commits into from
Oct 23, 2023

Conversation

valeriacaracciolo
Copy link
Contributor

@valeriacaracciolo valeriacaracciolo commented Sep 18, 2023

Project Abstract

We have discovered that integration tests for ink! contracts lack some of the functionalities, or present implementation differences, when compared to E2E testing. Integration tests run significantly faster than E2E (end-to-end) tests. If a full range of functionalities were provided, it could reduce testing and QA times. Our intention is to flatten the anvil of ink! integration testing. With a properly flattened anvil, quality tools can be built.

We have already conducted a comprehensive analysis to identify any missing functionalities in integration tests and implementation differences with E2E tests, and to propose and develop new testing features based on our findings. This analysis was carried as part of a previous grant (link).

Grant level

  • Level 1: Up to $10,000, 2 approvals
  • Level 2: Up to $30,000, 3 approvals
  • Level 3: Unlimited, 5 approvals (for >$100k: Web3 Foundation Council approval)

Application Checklist

  • The application template has been copied and aptly renamed (project_name.md).
  • I have read the application guidelines.
  • Payment details have been provided (bank details via email or BTC, Ethereum (USDC/DAI) or Polkadot/Kusama (USDT) address in the application).
  • The software delivered for this grant will be released under an open-source license specified in the application.
  • The initial PR contains only one commit (squash and force-push if needed).
  • The grant will only be announced once the first milestone has been accepted (see the announcement guidelines).
  • I prefer the discussion of this application to take place in a private Element/Matrix channel. My username is: @_______:matrix.org (change the homeserver if you use a different one)

@nikw3f nikw3f self-assigned this Sep 20, 2023
@nikw3f
Copy link
Contributor

nikw3f commented Sep 22, 2023

Hey team, thanks for the application. In my opinion $60k seems a bit expensive as in the original grant you asked for $40.5K for the complete delivery. Previous $13.5K grant has already been approved.

@valeriacaracciolo
Copy link
Contributor Author

valeriacaracciolo commented Sep 22, 2023 via email

@valeriacaracciolo
Copy link
Contributor Author

Hello @nikw3f we have identified a bug in the e2e tests. When contracts are in a workspace with dependencies defined in Cargo.toml, and these dependencies are inherited in contracts, the e2e tests fail to compile. However, manually specifying dependencies in each contract resolves the issue.
We've logged this bug on GitHub: Issue #1919. We'd like to address it as part of our grant work. Please confirm if we need to update our application or if we can proceed with the fix in our current work - once confirmed. Thank you! Best,

Copy link
Collaborator

@Noc2 Noc2 left a comment

Choose a reason for hiding this comment

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

Thanks for the quick reply here. It's best to integrate this into the deliverables of the application. Apart from this, I have one question: you estimate an FTE of 4, but you only shared 4 LinkedIn Profiles. Who is the fourth person in your team?

@Noc2 Noc2 added the changes requested The team needs to clarify a few things first. label Sep 25, 2023
@nikw3f
Copy link
Contributor

nikw3f commented Sep 25, 2023

Hello @nikw3f we have identified a bug in the e2e tests. When contracts are in a workspace with dependencies defined in Cargo.toml, and these dependencies are inherited in contracts, the e2e tests fail to compile. However, manually specifying dependencies in each contract resolves the issue. We've logged this bug on GitHub: Issue #1919. We'd like to address it as part of our grant work. Please confirm if we need to update our application or if we can proceed with the fix in our current work - once confirmed. Thank you! Best,

Yes please update the application.

@valeriacaracciolo
Copy link
Contributor Author

Hello @Noc2 and @nikw3f thanks for your reply. On this, we will do so.

Regarding the team, our company policy prioritizes confidentiality and doesn't typically disclose specific team assignments. However, in response to your inquiry, here is the team assigned to this project:
0.5 (PM) Cristian Paz Mezzano LinkedIn
1 Agustin Aón LinkedIn
1 Victor Gonzalez (not active on any social media)
0.5 Brian Ramirez LinkedIn
0.5 José García Crosta LinkedIn
0.5 Sofia Nicoletti LinkedIn

Ariel Wassbein, our Head of Research, will supervise the project and engage in pivotal moments as required. As for my role (Valeria Caracciolo), I am actively involved but in a peripheral capacity.

I hope the above helps clarify, we are happy to address any further inquiries.

@nikw3f nikw3f requested a review from Noc2 September 26, 2023 07:54
@nikw3f nikw3f added ready for review The project is ready to be reviewed by the committee members. and removed changes requested The team needs to clarify a few things first. labels Sep 26, 2023
keeganquigley
keeganquigley previously approved these changes Sep 26, 2023
Copy link
Contributor

@keeganquigley keeganquigley left a comment

Choose a reason for hiding this comment

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

Thanks @valeriacaracciolo for making the changes. I do also think that the price is a bit high, and this could be a point of contention for other committee members. But based on the quality of your previous work, I'm willing to go ahead and approve it at this rate. Looking forward to the results.

nikw3f
nikw3f previously approved these changes Sep 27, 2023
Copy link
Contributor

@nikw3f nikw3f left a comment

Choose a reason for hiding this comment

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

Thanks @valeriacaracciolo. Although i still feel price is high for this development, i am going to approve this. I am impressed with the previous report and would like to see the findings implemented for ink integration test infrastructure.

@valeriacaracciolo
Copy link
Contributor Author

Hi, wanted to inform you that, as we work on rectifying the end-to-end tests to operate within a workspace (Issue #1919), we are also addressing a few issues related to 'cargo-contract' (which also pertains to 'parity'). For the moment, there will be two pull requests: one to that repository and another to the 'ink' repository. Here are the issues that mention this error:

We believe that our last change to the Project Status and Deliverables in our proposal correctly specifies these types of add hoc developments needed for the implementation of the proposed improvements.

We plan to document each ad hoc modification in our delivery report, but let us know if you see an additional change to our proposal necessary for clarity.

@valeriacaracciolo
Copy link
Contributor Author

Hi everyone!

We came across a possible issue with e2e testing. We found that, when using the weight_to_fee function in e2e tests, the value of gas_price always returns zero. We provide a code example in the following link (https://github.com/CoinFabrik/on-ink-integration-tests/tree/d0c4deeb2b1d21f76d0020df7ab4330885c42da4/test-cases/weight-to-fee).

We plan to include this issue and the estimation for its resolution in our report of Milestone 2, but after validating with Michi (from partity) we where recommended to raise an issue in the respective repository. Do you concur?

@valeriacaracciolo
Copy link
Contributor Author

Hi @nikw3f @keeganquigley @Noc2

We came across a possible issue with e2e testing. Concretely we found that, when using the weight_to_fee function in e2e tests, the value of gas_price always returns zero. We provide a code example in the following link (https://github.com/CoinFabrik/on-ink-integration-tests/tree/d0c4deeb2b1d21f76d0020df7ab4330885c42da4/test-cases/weight-to-fee).

We plan to include this issue as part of our report of Milestone 1, since weight_to_fee was one of the functions pending analysis during this milestone. However, after talking to Michi from parity, he recommended us to post an issue to the corresponding repository. Do you concur?

@keeganquigley
Copy link
Contributor

Thanks @valeriacaracciolo yes feel free to file an issue in the repo since those are the engineers who will be working on it, and then you can link it in the delivery if necessary. Thanks for checking!

Noc2
Noc2 previously approved these changes Oct 1, 2023
Copy link
Member

@semuelle semuelle left a comment

Choose a reason for hiding this comment

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

Thanks for the application, @valeriacaracciolo, and sorry for the late response.
We have had issues with grants relying on merging PRs with other people's repositories, so I'd be in favor of starting with M1 for now, or a subset of the proposed functions anyway. However, I'll ping the rest of the committee again to see if they think differently.

@valeriacaracciolo
Copy link
Contributor Author

valeriacaracciolo commented Oct 13, 2023 via email

@takahser takahser self-requested a review October 18, 2023 10:23
We reduced the grant proposal to the Milestone Execution and Further Analysis, leaving the last milestone for a next proposal.

We also corrected a typo on the numbering of mentioned functions, and removed a comment on accounts that did not correspond.
@valeriacaracciolo valeriacaracciolo dismissed stale reviews from Noc2, nikw3f, and keeganquigley via 71dfdc2 October 23, 2023 16:39
@valeriacaracciolo
Copy link
Contributor Author

Hi @semuelle, we consulted with @keeganquigley and divided the milestones, keeping only the first one for this current application and leaving the second one for another grant application.

Please notice that we also changed the Level to 2.

Regards!

Copy link
Contributor

@keeganquigley keeganquigley left a comment

Choose a reason for hiding this comment

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

Thanks for the changes @valeriacaracciolo re-approving.

Copy link
Contributor

@nikw3f nikw3f left a comment

Choose a reason for hiding this comment

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

Good from my side

@Noc2 Noc2 merged commit a464436 into w3f:master Oct 23, 2023
7 checks passed
@github-actions
Copy link
Contributor

Congratulations and welcome to the Web3 Foundation Grants Program! Please refer to our Milestone Delivery repository for instructions on how to submit milestones and invoices, our FAQ for frequently asked questions and the support section of our README for more ways to find answers to your questions.

Before you start, take a moment to read through our announcement guidelines for all communications related to the grant or make them known to the right person in your organisation. In particular, please don't announce the grant publicly before at least the first milestone of your project has been approved. At that point or shortly before, you can get in touch with us at [email protected] and we'll be happy to collaborate on an announcement about the work you’re doing.

Lastly, please remember to let us know in case you run into any delays or deviate from the deliverables in your application. You can either leave a comment here or directly request to amend your application via PR. We wish you luck with your project! 🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready for review The project is ready to be reviewed by the committee members.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants