Skip to content
This repository was archived by the owner on Jul 5, 2024. It is now read-only.

Fix testool manifest and add test it in github actions#1535

Merged
adria0 merged 2 commits into
mainfrom
fix/testool-after-1524
Jul 21, 2023
Merged

Fix testool manifest and add test it in github actions#1535
adria0 merged 2 commits into
mainfrom
fix/testool-after-1524

Conversation

@adria0
Copy link
Copy Markdown
Contributor

@adria0 adria0 commented Jul 19, 2023

Description

Testool failed to compile after #1524, this is due:

  • testool compilation is not checked in github actions
  • some functions required are not only available without test-circuits feature

Type of change

Bug fix (non-breaking change which fixes an issue)

Contents

  • Fixes Cargo.toml
  • Adds basic test of testool in github actions

@github-actions github-actions Bot added the CI Issues related to the Continuous Integration mechanisms of the repository. label Jul 19, 2023
@adria0 adria0 changed the title fix(testool,gitactions): fix testool manifest and add test it in github actions Fix testool manifest and add test it in github actions Jul 19, 2023
@hero78119 hero78119 self-requested a review July 20, 2023 04:32
Copy link
Copy Markdown
Contributor

@hero78119 hero78119 left a comment

Choose a reason for hiding this comment

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

Hi @adria0 new added CI step is failed

---- compiler::test::test_docker_solidity stdout ----
Error: docker ["run", "-i", "--rm", "solc", "--standard-json", "-"] failed "Unable to find image 'solc:latest' locally\ndocker: Error response from daemon: pull access denied for solc, repository does not exist or may require 'docker login': denied: requested access to the resource is denied.\nSee 'docker run --help'.\n" when compiling >>>"{\"language\":\"Solidity\",\"settings\":{\"optimizer\":{\"enabled\":false,\"details\":{\"jumpdestRemover\":false,\"peephole\":false,\"inliner\":false}},\"outputSelection\":{\"*\":{\"*\":[\"evm.bytecode\"]}}},\"sources\":{\"stdin\":{\"content\":\"contract A{}\"}}}"<<<

---- compiler::test::test_docker_yul stdout ----
Error: docker ["run", "-i", "--rm", "solc", "--standard-json", "-"] failed "Unable to find image 'solc:latest' locally\ndocker: Error response from daemon: pull access denied for solc, repository does not exist or may require 'docker login': denied: requested access to the resource is denied.\nSee 'docker run --help'.\n" when compiling >>>"{\"language\":\"Yul\",\"settings\":{\"optimizer\":{\"enabled\":false,\"details\":{\"jumpdestRemover\":false,\"peephole\":false,\"inliner\":false}},\"outputSelection\":{\"*\":{\"*\":[\"evm.bytecode\"]}}},\"sources\":{\"stdin\":{\"content\":\"\\n{\\n    function power(base, exponent) -> result\\n    {\\n        result := 1\\n        for { let i := 0 } lt(i, exponent) { i := add(i, 1) }\\n        {\\n            result := mul(result, base)\\n        }\\n    }\\n}\\n            \"}}}"<<<

---- compiler::test::test_docker_lll stdout ----
Error: docker ["run", "-i", "--rm", "lllc"] failed "Unable to find image 'lllc:latest' locally\ndocker: Error response from daemon: pull access denied for lllc, repository does not exist or may require 'docker login': denied: requested access to the resource is denied.\nSee 'docker run --help'.\n" when compiling >>>"[[0]] (+ 0xffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff 4)"<<<


failures:
    compiler::test::test_docker_lll
    compiler::test::test_docker_solidity
    compiler::test::test_docker_yul

Seems the reason is need to execute testtool/test-docker.sh in advanced to build the docker image locally

@adria0 adria0 force-pushed the fix/testool-after-1524 branch from ab1f790 to ebbf695 Compare July 20, 2023 09:49
@adria0
Copy link
Copy Markdown
Contributor Author

adria0 commented Jul 20, 2023

Hey @hero78119 , thanks for checking this!
Maybe testing & building docker with this is too much, with --features="ignore-test-docker" should be enough, updated in ebbf695

Copy link
Copy Markdown
Contributor

@hero78119 hero78119 left a comment

Choose a reason for hiding this comment

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

LGTM!

hey @adria0, noticed test-docker.sh also removed, which might lost the information for how to build the docker to run locally. If docker still need, should we docs the build step in README.md as pre-setup step ?

@adria0
Copy link
Copy Markdown
Contributor Author

adria0 commented Jul 21, 2023

LGTM!

hey @adria0, noticed test-docker.sh also removed, which might lost the information for how to build the docker to run locally. If docker still need, should we docs the build step in README.md as pre-setup step ?

Yep, agree!

@adria0 adria0 enabled auto-merge July 21, 2023 07:25
@adria0 adria0 disabled auto-merge July 21, 2023 07:26
Copy link
Copy Markdown
Contributor

@ed255 ed255 left a comment

Choose a reason for hiding this comment

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

LGTM!

@adria0 adria0 added this pull request to the merge queue Jul 21, 2023
Merged via the queue into main with commit 350a7a2 Jul 21, 2023
@adria0 adria0 deleted the fix/testool-after-1524 branch July 21, 2023 11:07
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

CI Issues related to the Continuous Integration mechanisms of the repository.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants