Skip to content

Commit

Permalink
Update PRs in CoinFabrik_On_Ink_Integration_Tests-2.md
Browse files Browse the repository at this point in the history
We explain below two changes that we had to perform to ensure that our PRs passed the corresponding checks, and to be consistent with new developments we have performed on other unimplemented functions in integration tests.

We initially performed the Pull Request [#1963](use-ink/ink#1963) to implement instantiate_contract(), but decided to close it and perform a new Pull Request [#1988](use-ink/ink#1988) to include necessary implementations of other related functions, which were developed as a subsequent milestone to this Milestone 2. These functions were all related to each other, and a single PR seemed like the tidiest option.

As part of an ad-hoc development in PR [#58] (use-ink/cargo-contract#1358), adding support to workspaces, we were requested to ensure that ink-examples worked correctly with this implementation. We initially performed the Pull Request [#44](use-ink/ink-examples#44) to show this. However, we had to close this PR because some modifications needed to be performed for checks to pass. We made these changes in Pull Request [#52](use-ink/ink-examples#52).

We updated the Milestone Report and Table of Deliveries to reflect these final changes.
  • Loading branch information
valeriacaracciolo authored Nov 16, 2023
1 parent f2e936f commit 76a7092
Showing 1 changed file with 2 additions and 2 deletions.
4 changes: 2 additions & 2 deletions deliveries/CoinFabrik_On_Ink_Integration_Tests-2.md
Original file line number Diff line number Diff line change
Expand Up @@ -23,9 +23,9 @@ We documented our work in a [milestone report](https://github.com/CoinFabrik/on-
| 0c. | Testing and Testing Guide | - | We followed existing [contribution guidelines](https://github.com/paritytech/ink/blob/master/CONTRIBUTING.md) for the pull requests associated with the implementations of functions: default_accounts(), set_contract_storage() and instantiate_contract(). For these functions, we added test cases in their target directories correspondingly. We include a testing guide on how to execute these tests in [our report](https://github.com/CoinFabrik/on-ink-integration-tests/blob/milestone-on-ink-integration-tests-2/assets/On-Ink-Integration-Tests-2-Milestone-Report.pdf), on `Reference 1: Implementation Summary` of section `Execution` of the `Appendix 1`, and in the documentation of each corresponding pull request.
| 0d. | Docker | - | Does not apply at this stage.
| 0e. | Article | https://www.coinfabrik.com/blog/implementing-missing-functionalities-on-ink-integration-tests | We prepared a summary report and published it on our blog https://blog.coinfabrik.com/ under the name `Implementing Missing Functionalities On Ink Integration Tests`.
**1** | Develop | [PR1-default_accounts()](https://github.com/paritytech/ink/pull/1955), [PR2-set-account_storage()](https://github.com/paritytech/ink/pull/1961), [PR3-instantiate-contract()](https://github.com/paritytech/ink/pull/1963) | We provide implementations for the three functions specified in this milestone: [default_accounts()](https://github.com/paritytech/ink/pull/1955), [set_contract_storage()](https://github.com/paritytech/ink/pull/1961) and [instantiate_contract()](https://github.com/paritytech/ink/pull/1963). Check further documentation on these developments in [our report](https://github.com/CoinFabrik/on-ink-integration-tests/blob/milestone-on-ink-integration-tests-2/assets/On-Ink-Integration-Tests-2-Milestone-Report.pdf), in the `Reference 1: Implementation Summary` of the section `Execution` of the `Appendix 1`.<br>On the other hand, as we worked on integrating our test cases into CI/CD, we identified a bug when building e2e tests in a workspace. Concretely, we noticed that the build for e2e tests failed when running cargo-contract inside a workspace package, and if any dependency was inherited from the workspace definition. We performed a pull request to the cargo-contract repository implementing a [fix](https://github.com/paritytech/cargo-contract/pull/1358) to this issue. Check further documentation on this development in [our report](https://github.com/CoinFabrik/on-ink-integration-tests/blob/milestone-on-ink-integration-tests-2/assets/On-Ink-Integration-Tests-2-Milestone-Report.pdf), in the subsection `Ad-hoc developments` of the section `Execution` of the `Appendix 1`.
**1** | Develop | [PR1-default_accounts()](https://github.com/paritytech/ink/pull/1955), [PR2-set-account_storage()](https://github.com/paritytech/ink/pull/1961), [PR3-instantiate-contract()](https://github.com/paritytech/ink/pull/1988) | We provide implementations for the three functions specified in this milestone: [default_accounts()](https://github.com/paritytech/ink/pull/1955), [set_contract_storage()](https://github.com/paritytech/ink/pull/1961) and [instantiate_contract()](https://github.com/paritytech/ink/pull/1988). Check further documentation on these developments in [our report](https://github.com/CoinFabrik/on-ink-integration-tests/blob/milestone-on-ink-integration-tests-2/assets/On-Ink-Integration-Tests-2-Milestone-Report.pdf), in the `Reference 1: Implementation Summary` of the section `Execution` of the `Appendix 1`.<br>On the other hand, as we worked on integrating our test cases into CI/CD, we identified a bug when building e2e tests in a workspace. Concretely, we noticed that the build for e2e tests failed when running cargo-contract inside a workspace package, and if any dependency was inherited from the workspace definition. We performed a pull request to the cargo-contract repository implementing a [fix](https://github.com/paritytech/cargo-contract/pull/1358) to this issue. Check further documentation on this development in [our report](https://github.com/CoinFabrik/on-ink-integration-tests/blob/milestone-on-ink-integration-tests-2/assets/On-Ink-Integration-Tests-2-Milestone-Report.pdf), in the subsection `Ad-hoc developments` of the section `Execution` of the `Appendix 1`.
**2** | Review and Estimate | https://github.com/CoinFabrik/on-ink-integration-tests/blob/milestone-on-ink-integration-tests-2/assets/On-Ink-Integration-Tests-2-Milestone-Report.pdf | We performed a review of the 13 functions with pending analysis and found implementation differences between integration and e2e tests for the functions balance() and weight_to_fee(). For balance() we provide an implementation estimate. For weigh_to_fee() we provide documentation of the problem and an associated test case; the issue occurs on the e2e environment and needs to be discussed with the responsible team before moving forward. Check documentation of these reviews and estimates in our report, on `Reference 2: Findings and Estimation` of section `Further Analysis and Estimation` of the `Appendix 1`.
**3** | Quality Assurance | - | We reviewed and followed the established [contribution guidelines](https://github.com/paritytech/ink/blob/master/CONTRIBUTING.md) for the three pull requests performed: [PR1-default_accounts()](https://github.com/paritytech/ink/pull/1955), [PR2-set-account_storage()](https://github.com/paritytech/ink/pull/1961), [PR3-instantiate-contract()](https://github.com/paritytech/ink/pull/1963).
**3** | Quality Assurance | - | We reviewed and followed the established [contribution guidelines](https://github.com/paritytech/ink/blob/master/CONTRIBUTING.md) for the three pull requests performed: [PR1-default_accounts()](https://github.com/paritytech/ink/pull/1955), [PR2-set-account_storage()](https://github.com/paritytech/ink/pull/1961), [PR3-instantiate-contract()](https://github.com/paritytech/ink/pull/1988).



Expand Down

0 comments on commit 76a7092

Please sign in to comment.