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

Fix broken Github Actions environments of main branch #290

Merged
merged 18 commits into from
Jul 28, 2021

Conversation

tnasu
Copy link
Member

@tnasu tnasu commented Jul 9, 2021

Relate: #289

Description

Fix the below:

  • lint.yaml
  • linter.yml
  • tests.yml
  • coverage.yml
  • e2e.yml
    • f0c61e3: Updating maverick node is difficult, so I decided to separate the correspondence from this PR.
    • After commits are for e2e-test

1bb7c29: In addition, I changed the original behavior of notifyTxsAvailable because I think there is a timing issue when it calls, and I don't think panic is a good behavior

@CLAassistant
Copy link

CLAassistant commented Jul 9, 2021

CLA assistant check
All committers have signed the CLA.

@tnasu tnasu added the WIP label Jul 9, 2021
@tnasu tnasu changed the title Fix broken Github Actions environments of main branch WIP: Fix broken Github Actions environments of main branch Jul 9, 2021
@tnasu
Copy link
Member Author

tnasu commented Jul 9, 2021

lint.yaml

[Lint/golangci-lint]   ❗  ::error file=consensus/replay_test.go,line=351,col=6::func `consensusNewBlock` is unused (unused)
[Lint/golangci-lint]   ❗  ::error file=consensus/replay_test.go,line=858,col=6::func `buildTMStateFromChain` is unused (unused)
[Lint/golangci-lint]   ❗  ::error file=consensus/replay_test.go,line=333,col=6::func `createProposalBlock` is unused (unused)
[Lint/golangci-lint]   ❗  ::error file=state/state_test.go,line=480,col=6::func `testProposerFreq` is unused (unused)
[Lint/golangci-lint]   ❗  ::error file=consensus/common_test.go,line=152,col=6::type `ValidatorStubsByPower` is unused (unused)
[Lint/golangci-lint]   ❗  ::error file=consensus/replay_test.go,line=313,col=2::var `mempool` is unused (unused)
[Lint/golangci-lint]   ❗  ::error file=consensus/replay_test.go,line=817,col=6::func `buildAppStateFromChain` is unused (unused)
[Lint/golangci-lint]   ❗  ::error file=consensus/replay_test.go,line=1156,col=6::func `readPieceFromWAL` is unused (unused)
[Lint/golangci-lint]   ❗  ::error file=types/validator_set_test.go,line=775,col=6::func `permutation` is unused (unused)
[Lint/golangci-lint]   ❗  ::error file=consensus/replay_test.go,line=326,col=5::var `modes` is unused (unused)
[Lint/golangci-lint]   ❗  ::error file=consensus/replay_test.go,line=314,col=2::var `evpool` is unused (unused)
[Lint/golangci-lint]   ❗  ::error file=consensus/replay_test.go,line=316,col=2::var `sim` is unused (unused)
[Lint/golangci-lint]   ❗  ::error file=consensus/replay_test.go,line=1051,col=6::func `makeBlockchainFromWAL` is unused (unused)
[Lint/golangci-lint]   ❗  ::error file=consensus/replay_test.go,line=328,col=6::func `getProposerIdx` is unused (unused)
[Lint/golangci-lint]   ❗  ::error file=consensus/replay_test.go,line=675,col=6::func `testHandshakeReplay` is unused (unused)
[Lint/golangci-lint]   ❗  ::error file=consensus/replay_test.go,line=805,col=6::func `applyBlock` is unused (unused)

Can't fix unused in the "_test.go" since there is a bug in golangci-lint

We should upgrade golangci-lint but I did workaround like this:

args: --timeout 10m --skip-files "_test\.go"

In addition, tests of args is not work...

@tnasu tnasu force-pushed the main-fix-minnor-issues branch from 0fcd279 to 47f98c0 Compare July 9, 2021 08:27
@tnasu
Copy link
Member Author

tnasu commented Jul 9, 2021

tests.yml

...

| [warning]Cache Service Url not found, unable to restore cache.
[Tests/test_apps]   ⚙  ::set-output:: cache-hit=false

...

| Running all
| + kvstore_over_socket
| + rm -rf /root/.ostracon_app
| + ostracon init
| test/app/test.sh: line 16: ostracon: command not found
[Tests/test_apps]   ❌  Failure - test_apps

...

Can't fix the missing cache-hit since there is a bug in action/cache

I did workaround for each jobs(test_abci_apps,test_abci_cli,test_apps) like this:

       - uses: actions/cache@v1
+         id: gobin-cache
         with:
           path: ~/go/bin
           key: ${{ runner.os }}-${{ github.sha }}-oc-binary
         if: env.GIT_DIFF
+       - name: Re-install when cannot get the cached binary
+         run: make install install_abci
+         if: steps.gobin-cache.outputs.cache-hit != 'true'

@codecov
Copy link

codecov bot commented Jul 9, 2021

Codecov Report

❗ No coverage uploaded for pull request base (main@a727fe1). Click here to learn what that means.
The diff coverage is n/a.

❗ Current head 981f699 differs from pull request most recent head 1bb7c29. Consider uploading reports for the commit 1bb7c29 to get more accurate results

@@           Coverage Diff           @@
##             main     #290   +/-   ##
=======================================
  Coverage        ?   61.22%           
=======================================
  Files           ?      273           
  Lines           ?    30013           
  Branches        ?        0           
=======================================
  Hits            ?    18374           
  Misses          ?     9959           
  Partials        ?     1680           

@tnasu tnasu force-pushed the main-fix-minnor-issues branch 4 times, most recently from 24ddc95 to d632ebf Compare July 20, 2021 10:02
@tnasu tnasu force-pushed the main-fix-minnor-issues branch from 7f2a03a to 4672023 Compare July 27, 2021 07:14
@tnasu tnasu force-pushed the main-fix-minnor-issues branch from a84b9ae to 1bb7c29 Compare July 27, 2021 10:16
@tnasu tnasu changed the title WIP: Fix broken Github Actions environments of main branch Fix broken Github Actions environments of main branch Jul 27, 2021
@tnasu tnasu removed the WIP label Jul 27, 2021
@tnasu tnasu requested review from torao and Kynea0b July 27, 2021 10:43
Copy link
Contributor

@Kynea0b Kynea0b left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@torao torao left a comment

Choose a reason for hiding this comment

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

LGTM

@tnasu tnasu merged commit 1132e00 into Finschia:main Jul 28, 2021
@tnasu tnasu self-assigned this Aug 13, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants