Skip to content
Merged
38 changes: 36 additions & 2 deletions .github/workflows/main.yml
Original file line number Diff line number Diff line change
Expand Up @@ -24,11 +24,30 @@ jobs:
- name: Check Format (*.cs)
run: dotnet format --verify-no-changes --verbosity diagnostic

Build-Test-Neo-Cli:
needs: [Format]
timeout-minutes: 15
runs-on: ubuntu-latest
steps:
- name: Checkout
uses: actions/checkout@v4

- name: Setup .NET
uses: actions/setup-dotnet@v4
with:
dotnet-version: ${{ env.DOTNET_VERSION }}

- name: Build (Neo.CLI)
run: dotnet build ./src/Neo.CLI --output ./out/Neo.CLI

- name: Install dependencies
run: |
dotnet build ./src/Neo.CLI \
--output ./out/Neo.CLI
sudo apt-get install libleveldb-dev expect
Comment thread
vncoelho marked this conversation as resolved.
find ./out -name 'config.json' | xargs perl -pi -e 's|LevelDBStore|MemoryStore|g'

- name: Run tests with expect
run: expect ./src/Neo.CLI/Scripts/test-neo-cli.expect

Test:
needs: [Format]
timeout-minutes: 15
Expand All @@ -39,15 +58,18 @@ jobs:
steps:
- name: Checkout
uses: actions/checkout@v4

- name: Setup .NET
uses: actions/setup-dotnet@v4
with:
dotnet-version: ${{ env.DOTNET_VERSION }}

- name: Test
if: matrix.os != 'ubuntu-latest'
run: |
dotnet sln neo.sln remove ./tests/Neo.Plugins.Storage.Tests/Neo.Plugins.Storage.Tests.csproj
dotnet test

- name: Test for coverall
if: matrix.os == 'ubuntu-latest'
run: |
Expand Down Expand Up @@ -139,14 +161,17 @@ jobs:
steps:
- name: Checkout
uses: actions/checkout@v4

- name: Get version
id: get_version
run: |
sudo apt install xmlstarlet
find src -name Directory.Build.props | xargs xmlstarlet sel -N i=http://schemas.microsoft.com/developer/msbuild/2003 -t -v "concat('::set-output name=version::v',//i:VersionPrefix/text())" | xargs echo

- name: Check tag
id: check_tag
run: curl -s -I ${{ format('https://github.com/{0}/releases/tag/{1}', github.repository, steps.get_version.outputs.version) }} | head -n 1 | cut -d$' ' -f2 | xargs printf "::set-output name=statusCode::%s" | xargs echo

- name: Create release
if: steps.check_tag.outputs.statusCode == '404'
id: create_release
Expand All @@ -157,53 +182,62 @@ jobs:
tag_name: ${{ steps.get_version.outputs.version }}
release_name: ${{ steps.get_version.outputs.version }}
prerelease: ${{ contains(steps.get_version.outputs.version, '-') }}

- name: Setup .NET
if: steps.check_tag.outputs.statusCode == '404'
uses: actions/setup-dotnet@v4
with:
dotnet-version: ${{ env.DOTNET_VERSION }}

- name : Pack (Neo)
if: steps.check_tag.outputs.statusCode == '404'
run: |
dotnet pack ./src/Neo \
--configuration Release \
--output ./out

- name : Pack (Neo.IO)
if: steps.check_tag.outputs.statusCode == '404'
run: |
dotnet pack ./src/Neo.IO \
--configuration Release \
--output ./out

- name : Pack (Neo.Extensions)
if: steps.check_tag.outputs.statusCode == '404'
run: |
dotnet pack ./src/Neo.Extensions \
--configuration Release \
--output ./out

- name : Pack (Neo.Json)
if: steps.check_tag.outputs.statusCode == '404'
run: |
dotnet pack ./src/Neo.Json \
--configuration Release \
--output ./out

- name : Pack (Neo.VM)
if: steps.check_tag.outputs.statusCode == '404'
run: |
dotnet pack ./src/Neo.VM \
--configuration Release \
--output ./out

- name : Pack (Neo.ConsoleService)
if: steps.check_tag.outputs.statusCode == '404'
run: |
dotnet pack ./src/Neo.ConsoleService \
--configuration Release \
--output ./out

- name : Pack (Neo.Cryptography.BLS12_381)
if: steps.check_tag.outputs.statusCode == '404'
run: |
dotnet pack ./src/Neo.Cryptography.BLS12_381 \
--configuration Release \
--output ./out

- name: Publish to NuGet
if: steps.check_tag.outputs.statusCode == '404'
run: |
Expand Down
86 changes: 86 additions & 0 deletions src/Neo.CLI/Scripts/test-neo-cli.expect
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

We don't scripts included the csproj files. Just create a folder /scripts/Neo.CLI and put there. If doesn't make sense to put in with source files.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

You can fix it in another pr. If @vncoelho 's purpose is bring this back to the project. Please let him just bring it back.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I'm agree with do it organized, why we need to create one pr for one thing and other for move it? review is for this, the solution is not create a new pr.

Copy link
Copy Markdown
Contributor

@Jim8y Jim8y Jun 12, 2024

Choose a reason for hiding this comment

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

I'm agree with do it organized, why we need to create one pr for one thing and other for move it? review is for this, the solution is not create a new pr.

The problem is this pr from @vncoelho is to bring expert back, moving it somewhere is a request from @cschuchardt88 , which is not what @vncoelho wanted to do..... Why would you require/enforce @vncoelho to do a thing that is out of the scope of his pr? I am not against of making it organized, but what if @vncoelho just not willing to apply his suggestion? what we do with this pr then? close it and open another one?

So my point is very simple, one pr for only one task, if you agree that we should bring expert back, and @vncoelho 's pr has fullfilled this purpose, we should be fine with this pr. You can have suggestions, but that should not be a reason of enforcing unless there is a real problem in the pr.

From our past working experience, i believe having small prs (real pr that can fix problem without introducing new problem), is the only way we can make it forward.

BUT, its just my personal suggestion.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

His suggestion is good anyway, I will move to the main folder.

Copy link
Copy Markdown
Member Author

@vncoelho vncoelho Jun 12, 2024

Choose a reason for hiding this comment

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

Something I would like to highlighted is that @cschuchardt88 is using the Requested Changes too much.
I think that this is more useful when you are really blocking something for a good reason.
These uses like this are not necessary and just reduce the criteria we will often look to these requests.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I'm agree with do it organized, why we need to create one pr for one thing and other for move it? review is for this, the solution is not create a new pr.

The problem is this pr from @vncoelho is to bring expert back, moving it somewhere is a request from @cschuchardt88 , which is not what @vncoelho wanted to do..... Why would you require/enforce @vncoelho to do a thing that is out of the scope of his pr? I am not against of making it organized, but what if @vncoelho just not willing to apply his suggestion? what we do with this pr then? close it and open another one?

So my point is very simple, one pr for only one task, if you agree that we should bring expert back, and @vncoelho 's pr has fullfilled this purpose, we should be fine with this pr. You can have suggestions, but that should not be a reason of enforcing unless there is a real problem in the pr.

From our past working experience, i believe having small prs (real pr that can fix problem without introducing new problem), is the only way we can make it forward.

BUT, its just my personal suggestion.

I completely agree with you. This should be a common sense for most of the PRs.
Stop trying to block PRs for nonsense things that are not the goal of the PRs themselves.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I'm agree with do it organized, why we need to create one pr for one thing and other for move it? review is for this, the solution is not create a new pr.

The problem is this pr from @vncoelho is to bring expert back, moving it somewhere is a request from @cschuchardt88 , which is not what @vncoelho wanted to do..... Why would you require/enforce @vncoelho to do a thing that is out of the scope of his pr? I am not against of making it organized, but what if @vncoelho just not willing to apply his suggestion? what we do with this pr then? close it and open another one?

So my point is very simple, one pr for only one task, if you agree that we should bring expert back, and @vncoelho 's pr has fullfilled this purpose, we should be fine with this pr. You can have suggestions, but that should not be a reason of enforcing unless there is a real problem in the pr.

From our past working experience, i believe having small prs (real pr that can fix problem without introducing new problem), is the only way we can make it forward.

BUT, its just my personal suggestion.

Something I would like to highlighted is that @cschuchardt88 is using the Requested Changes too much. I think that this is more useful when you are really blocking something for a good reason. These uses like this are not necessary and just reduce the criteria we will often look to these requests.

Than you should be creating a Issue and assigning someone to that task. I can't keep being the repo cleaner upper. Put some thought into it, that's all. Just remember no one can do it the right way but yourself. No copy/paste and hope.

Original file line number Diff line number Diff line change
@@ -0,0 +1,86 @@
#!/usr/bin/expect -f
#
# This script uses expect to test neo-cli
#
set timeout 10


# Start neo-cli
spawn dotnet out/Neo.CLI/neo-cli.dll

# Expect the main input prompt
expect {
"neo> " { }
"error" { exit 2 }
timeout { exit 1 }
}

#
# Test 'create wallet'
#
send "create wallet test-wallet1.json\n"

expect {
"password:" { send "asd\n" }
"error" { exit 2 }
timeout { exit 1 }
}

expect {
"password:" { send "asd\n" }
"error" { exit 2 }
timeout { exit 1 }
}

expect {
" Address:" { }
"error" { exit 2 }
timeout { exit 1 }
}


#
# Test 'create wallet'
#
send "create wallet test-wallet2.json L2ArHTuiDL4FHu4nfyhamrG8XVYB4QyRbmhj7vD6hFMB5iAMSTf6\n"

expect {
"password:" { send "abcd\n" }
"error" { exit 2 }
timeout { exit 1 }
}

expect {
"password:" { send "abcd\n" }
"error" { exit 2 }
timeout { exit 1 }
}

expect {
"NUj249PQg9EMJfAuxKizdJwMG7GSBzYX2Y" { }
"error" { exit 2 }
timeout { exit 1 }
}

#
# Test 'list address'
#
send "list address\n"

expect {
"neo> " { }
"error" { exit 2 }
timeout { exit 1 }
}

#
# Test 'create address'
#
send "create address\n"

expect {
"neo> " { }
"error" { exit 2 }
timeout { exit 1 }
}
exit 0