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

Added check to forc_abi to fail on non-contracts #1105

Merged
merged 30 commits into from
Apr 4, 2022

Conversation

eureka-cpu
Copy link
Contributor

@eureka-cpu eureka-cpu commented Mar 30, 2022

Closes #526 -- Initially reused the logic found in forc_deploy & forc_run to throw an error instead of an empty abi [] if the wrong program type was found.

Closes #1106 -- Replaced the CliError struct with anyhow::Error.

Closes #1117 -- Separated the tests in vec positive_project_names in e2e_vm_tests::mod into ...no_abi, ...with_abi, removed the json.oracle files and updated the surrounding functions to handle the aforementioned changes.

I've also converted the reused logic in forc_abi_json, forc_deploy & forc_run into a single function called check_program_type. To avoid circular dependency, I've moved it and its associated functions out of utils::cli_error and into forc_pkg::pkg since we decided to implement program_type as a method on manifest::Manifest. This helps by removing duplicate code which is being tracked here: #1107 .

@eureka-cpu eureka-cpu added bug Something isn't working forc labels Mar 30, 2022
@eureka-cpu eureka-cpu self-assigned this Mar 30, 2022
forc/src/utils/cli_error.rs Outdated Show resolved Hide resolved
forc/src/ops/forc_abi_json.rs Show resolved Hide resolved
@eureka-cpu
Copy link
Contributor Author

eureka-cpu commented Mar 31, 2022

@adlerjohn & @mitchmindtree getting this error on my machine even though CI passed:

Deploying should_pass/test_contracts/basic_storage
  Compiled library "core".
  Compiled library "std".
  Compiled library "basic_storage_abi".
  Compiled contract "basic_storage".
  Bytecode size is 108 bytes.
Contract id: 0xfa4d1b8c5dcbdd3a4ea073d162cb2313e89a03da03870a5561c12ffe10bca189
thread 'main' panicked at 'called `Result::unwrap()` on an `Err` value: An error occured while creating a new object: Connection refused (os error 61)', test/src/e2e_vm_tests/harness.rs:31:10

@eureka-cpu eureka-cpu requested review from mitchmindtree and sezna and removed request for emilyaherbert March 31, 2022 04:49
@mohammadfawaz
Copy link
Contributor

mohammadfawaz commented Mar 31, 2022

@adlerjohn & @mitchmindtree getting this error on my machine even though CI passed:


Deploying should_pass/test_contracts/basic_storage

  Compiled library "core".

  Compiled library "std".

  Compiled library "basic_storage_abi".

  Compiled contract "basic_storage".

  Bytecode size is 108 bytes.

Contract id: 0xfa4d1b8c5dcbdd3a4ea073d162cb2313e89a03da03870a5561c12ffe10bca189

thread 'main' panicked at 'called `Result::unwrap()` on an `Err` value: An error occured while creating a new object: Connection refused (os error 61)', test/src/e2e_vm_tests/harness.rs:31:10

Do you have fuel-core running? If so, try to clean up ~/.forc and ~./fuel and then relaunch it.

@eureka-cpu eureka-cpu reopened this Mar 31, 2022
@eureka-cpu
Copy link
Contributor Author

Should you remove the JSON ABI oracle files for scripts in the test suite?

accidentally closed while trying to quote reply 🤦🏻 yes, I'll remove those rn thank you!

@eureka-cpu eureka-cpu requested a review from adlerjohn April 1, 2022 00:19
forc/src/cli/commands/run.rs Show resolved Hide resolved
forc/src/ops/forc_deploy.rs Show resolved Hide resolved
forc/src/ops/forc_deploy.rs Outdated Show resolved Hide resolved
forc/src/utils/cli_error.rs Outdated Show resolved Hide resolved
sway-core/src/semantic_analysis/syntax_tree.rs Outdated Show resolved Hide resolved
test/src/e2e_vm_tests/mod.rs Show resolved Hide resolved
test/src/e2e_vm_tests/mod.rs Outdated Show resolved Hide resolved
@eureka-cpu eureka-cpu requested a review from sezna April 1, 2022 03:58
forc/src/ops/forc_deploy.rs Outdated Show resolved Hide resolved
forc/src/utils/cli_error.rs Outdated Show resolved Hide resolved
…ions to accept TreeType and updated docstrings
@eureka-cpu eureka-cpu requested a review from mitchmindtree April 2, 2022 05:26
@eureka-cpu eureka-cpu requested a review from otrho April 2, 2022 06:12
Copy link
Contributor

@mitchmindtree mitchmindtree left a comment

Choose a reason for hiding this comment

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

Getting closer!

forc/src/ops/forc_run.rs Outdated Show resolved Hide resolved
forc-pkg/src/manifest.rs Show resolved Hide resolved
forc-pkg/src/pkg.rs Outdated Show resolved Hide resolved
forc/src/ops/forc_abi_json.rs Show resolved Hide resolved
@eureka-cpu eureka-cpu requested a review from mitchmindtree April 2, 2022 07:43
forc-pkg/src/manifest.rs Outdated Show resolved Hide resolved
@eureka-cpu eureka-cpu requested a review from mitchmindtree April 2, 2022 07:51
sezna
sezna previously approved these changes Apr 4, 2022
Copy link
Contributor

@sezna sezna left a comment

Choose a reason for hiding this comment

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

approving to dismiss my requested changes but deferring to @mitchmindtree

Copy link
Contributor

@mitchmindtree mitchmindtree left a comment

Choose a reason for hiding this comment

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

Epic work @eureka-cpu, I think this is good to go!

@eureka-cpu eureka-cpu merged commit 264f60b into master Apr 4, 2022
@eureka-cpu eureka-cpu deleted the eureka-cpu/check-forc-abi-type branch April 4, 2022 22:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working forc
Projects
Archived in project
6 participants