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

check/fix ZEBRA_SKIP_NETWORK_TESTS #1730

Closed
oxarbitrage opened this issue Feb 12, 2021 · 7 comments · Fixed by #1732
Closed

check/fix ZEBRA_SKIP_NETWORK_TESTS #1730

oxarbitrage opened this issue Feb 12, 2021 · 7 comments · Fixed by #1732
Assignees
Labels
A-devops Area: Pipelines, CI/CD and Dockerfiles A-rust Area: Updates to Rust code C-bug Category: This is a bug
Milestone

Comments

@oxarbitrage
Copy link
Contributor

Some tests are skipped from the CI:

# Ubuntu runners don't have network or DNS configured during test steps
# Windows runners have an unreliable network
if: matrix.os == 'ubuntu-latest' || matrix.os == 'windows-latest'
run: echo "ZEBRA_SKIP_NETWORK_TESTS=1" >> $GITHUB_ENV

The comment is not true and we have a more stable zebra now. First check if it an simply be removed completely. If this is not possible check the tests that use ZEBRA_SKIP_NETWORK_TESTS individually and skip only the ones that will not pass. Explain why they need to be skipped.

This issue is a follow-up of #1726

@oxarbitrage oxarbitrage added C-bug Category: This is a bug S-needs-triage Status: A bug report needs triage labels Feb 12, 2021
@oxarbitrage
Copy link
Contributor Author

oxarbitrage commented Feb 13, 2021

The comment seems to be actually true, i made several tests in #1732 and we dont have any network capability: https://github.com/ZcashFoundation/zebra/pull/1732/checks?check_run_id=1891138982#step:6:7

It is a bit strange that in the docs they refer to ping or traceroute however other networking commands including dns resolution are not working either: https://docs.github.com/en/actions/reference/specifications-for-github-hosted-runners#cloud-hosts-for-github-hosted-runners and that is our problem.

Inbound ICMP packets are blocked for all Azure virtual machines, so ping or traceroute commands might not work.

From the same documentation:

GitHub hosts macOS runners in GitHub's own macOS Cloud.

This seems to be the reason of why macOS is actually working.

I think there is not much we can do than to update a bit the reason of the skipped tests in the comment.

@teor2345
Copy link
Contributor

We should also explain why the other tests that run zebrad work.

Most tests only check launch behaviour (or local port binding). After those tests succeed, zebrad hangs until its network requests time out. But since the tests don't check any output past port binding, it doesn't matter whether zebrad hangs or continues.

@teor2345
Copy link
Contributor

Hey @oxarbitrage, it looks like Windows CI builds are still running the large sync tests:
https://github.com/ZcashFoundation/zebra/pull/1750/checks?check_run_id=1908798434#step:8:559
https://github.com/ZcashFoundation/zebra/pull/1749/checks?check_run_id=1908787726#step:8:559

So there are a few issues with this ticket:

These are all draft PRs, please feel free to edit or replace them.

I'm pretty much at the end of my day right now, but maybe @dconnolly can help you sort out these CI failures.

@teor2345 teor2345 added A-devops Area: Pipelines, CI/CD and Dockerfiles A-rust Area: Updates to Rust code P-High labels Feb 16, 2021
@teor2345 teor2345 added this to the 2021 Sprint 3 milestone Feb 16, 2021
@teor2345
Copy link
Contributor

Maybe we need powershell syntax to set Windows env vars?
https://docs.github.com/en/actions/reference/workflow-commands-for-github-actions#environment-files

@teor2345
Copy link
Contributor

Or maybe we need to deploy more Zebra instances to testnet (#1222, blocked by #1709).

@oxarbitrage
Copy link
Contributor Author

The problem here is that i did:

if: matrix.os != 'macOS-latest'

And this seems to skip the env var setup fully. The var will only activate when the OS is macOS. I don't know how this actually passed some tests before but anyway i think the way to go it is like we had before:

if: matrix.os == 'ubuntu-latest' || matrix.os == 'windows-latest'

This is done at #1752

@oxarbitrage
Copy link
Contributor Author

Closed by #1752

@mpguerra mpguerra removed the S-needs-triage Status: A bug report needs triage label Feb 18, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-devops Area: Pipelines, CI/CD and Dockerfiles A-rust Area: Updates to Rust code C-bug Category: This is a bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants