test: Rework ci (Use travis only as fallback env) #16582

MarcoFalke commented at 4:15 PM on August 10, 2019: member

This moves the .travis folder to ci and removes dependence on travis, so that the test script can be run anywhere.

DrahtBot added the label Docs on Aug 10, 2019

DrahtBot added the label Tests on Aug 10, 2019

practicalswift commented at 6:04 PM on August 10, 2019: contributor

@MarcoFalke Could you provide some additional context? With this PR merged tests would still run in both Appveyor and Travis, right? What is the plan? :-)

MarcoFalke commented at 1:32 AM on August 11, 2019: member

The plan is to be flexible, to be able to switch away from travis any time without much hassle. Also, this makes it easier to run the ci locally.

in ci/README.md:5 in 9444777374 outdated

   0 | @@ -0,0 +1,8 @@
   1 | +## ci scripts
   2 | +
   3 | +This directory contains scripts for each build step in each build stage.
   4 | +
   5 | +Currently three stages `lint`, `extended_lint` and `test` are defined. Each stage has it's own lifecycle, similar to the

sipa commented at 2:25 AM on August 11, 2019:

its

MarcoFalke force-pushed on Aug 11, 2019

promag commented at 1:48 PM on August 11, 2019: member

Concept ACK.

MarcoFalke force-pushed on Aug 11, 2019

MarcoFalke removed the label Docs on Aug 11, 2019

MarcoFalke force-pushed on Aug 11, 2019

MarcoFalke commented at 3:17 PM on August 11, 2019: member

Fixed typo in doc and added a note on how to run the ci locally

MarcoFalke force-pushed on Aug 11, 2019

DrahtBot commented at 4:44 PM on August 11, 2019: member

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Conflicts

Reviewers, this pull request conflicts with the following ones:

#16597 ([WIP] Travis: run full test suite on native macOS by Sjors)
#12134 (Build previous releases and run functional tests by Sjors)

If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

MarcoFalke force-pushed on Aug 11, 2019

in ci/README.md:15 in fa6382cc4d outdated

  10 | +
  11 | +To allow for a wide range of tested environments, but also ensure reproducibility to some extent, the test stage
  12 | +requires `docker` to be installed. To install all requirements on Ubuntu, run
  13 | +
  14 | +```
  15 | +sudo apt install docker.io ccache bash git

fanquake commented at 12:53 AM on August 12, 2019:

I'd rather we just have a note to look at the official docs for how to install docker-ce. docker.io is an "old" version and the docs recommend uninstalling it.

MarcoFalke commented at 11:50 AM on August 12, 2019:

Huh? According to https://packages.ubuntu.com/xenial/docker.io it is the latest version. We don't need any fancy new features (other than --mount), so it should be fine as is.

fanquake commented at 11:58 AM on August 12, 2019:

Interesting. From the Docker documentation:

Older versions of Docker were called docker, docker.io , or docker-engine. If these are installed, uninstall them:
sudo apt-get remove docker docker-engine docker.io containerd runc

I guess I don't mind either way, just wasn't sure after looking at those docs.

in ci/README.md:6 in fa6382cc4d outdated

   0 | @@ -0,0 +1,24 @@
   1 | +## ci scripts
   2 | +
   3 | +This directory contains scripts for each build step in each build stage.
   4 | +
   5 | +Currently three stages `lint`, `extended_lint` and `test` are defined. Each stage has its own lifecycle, similar to the
   6 | +[Travis CI lifecycle](https://docs.travis-ci.com/user/job-lifecycle#the-job-lifecycle).  Every script in here is named

fanquake commented at 12:55 AM on August 12, 2019:

nit: extra space

in ci/test/04_install.sh:9 in fa6382cc4d outdated

   0 | @@ -0,0 +1,49 @@
   1 | +#!/usr/bin/env bash
   2 | +#
   3 | +# Copyright (c) 2018 The Bitcoin Core developers
   4 | +# Distributed under the MIT software license, see the accompanying
   5 | +# file COPYING or http://www.opensource.org/licenses/mit-license.php.
   6 | +
   7 | +export LC_ALL=C.UTF-8
   8 | +
   9 | +ccache echo "Create ccache dir, if it didn't exist"

fanquake commented at 1:18 AM on August 12, 2019:

ccache echo "Creating ccache dir if it didn't already exist"

fanquake commented at 2:53 AM on August 12, 2019: member

Concept ACK.

Have tested running the CI locally on macOS using Docker and ./ci/test_run_all.sh. Everything seems to work as expected.

It's a little annoying that we have to bring more bash scripts into this repo, but I guess they can just sit, and we wont have to modify/touch them at all.

edit: Could also add out/ and qa-assets/ to .gitignore.

MarcoFalke force-pushed on Aug 12, 2019

MarcoFalke commented at 12:06 PM on August 12, 2019: member

Have tested running the CI locally on macOS using Docker and ./ci/test_run_all.sh. Everything seems to work as expected.

Thanks for testing on mac!

So this is confirmed to be working on:

Locally, macOS host (fanquake)
travis, xenial host (run as part of this pull)
cirrus ci, bionic host (https://cirrus-ci.com/task/5077011800784896)
GitHub ci, bionic host (https://github.com/MarcoFalke/bitcoin/runs/190746548)
Locally, bionic host (MarcoFalke)

instagibbs commented at 1:08 PM on August 12, 2019: member

Also, this makes it easier to run the ci locally.

Concept ACK!

in ci/retry/retry:20 in faafa4739a outdated

  15 | +
  16 | +__log_out() {
  17 | +  echo "$1" 1>&2
  18 | +}
  19 | +
  20 | +# Paramters: max_tries min_sleep max_sleep constant_sleep fail_script EXECUTION_COMMAND

practicalswift commented at 7:37 AM on August 14, 2019:

Should be "parameters" :-)

fanquake commented at 7:47 AM on August 14, 2019:

This is an upstream script, so we don't need to fix it's issues here. Regardless, I've PR'd them upstream.

in ci/retry/retry:86 in faafa4739a outdated

  81 | +Usage: $retry [options] -- execute command
  82 | +    -h, -?, --help
  83 | +    -v, --verbose                    Verbose output
  84 | +    -t, --tries=#                    Set max retries: Default 10
  85 | +    -s, --sleep=secs                 Constant sleep amount (seconds)
  86 | +    -m, --min=secs                   Exponenetial Backoff: minimum sleep amount (seconds): Default 0.3

practicalswift commented at 7:38 AM on August 14, 2019:

Should be exponential -- applies throughout PR :-)

in .cirrus.yml:30 in faafa4739a outdated

  26 | @@ -27,3 +27,26 @@ task:
  27 |      - gmake check ${MAKEJOBS} VERBOSE=1
  28 |    functional_test_script:
  29 |      - ./test/functional/test_runner.py --jobs 9 --ci --extended --exclude feature_dbcrash --combinedlogslen=1000 --quiet --failfast
  30 | +task:

practicalswift commented at 7:39 AM on August 14, 2019:

Duplication of key task in mapping?

MarcoFalke commented at 2:09 PM on August 14, 2019:

Yes, this is the correct syntax:

https://cirrus-ci.org/guide/writing-tasks/#task-execution-dependencies

in .cirrus.yml:47 in faafa4739a outdated

  41 | +    CCACHE_DIR: "/tmp/ccache_dir"
  42 | +  ccache_cache:
  43 | +    folder: "/tmp/ccache_dir"
  44 | +  depends_cache:
  45 | +    folder: "/tmp/cirrus-ci-build/depends/built"
  46 | +    folder: "/tmp/cirrus-ci-build/depends/sdk-sources"

practicalswift commented at 7:39 AM on August 14, 2019:

Duplication of key folder in mapping?

DrahtBot added the label Needs rebase on Aug 14, 2019

MarcoFalke force-pushed on Aug 14, 2019

MarcoFalke commented at 5:40 PM on August 14, 2019: member

Rebased with three-way-merge due to change in tangent line

DrahtBot removed the label Needs rebase on Aug 14, 2019

fanquake commented at 12:39 PM on August 15, 2019: member

I'm now seeing an error running this:

configure.ac:40: installing 'build-aux/missing'
src/Makefile.am: installing 'build-aux/depcomp'
travis_fold:end:autogen

travis_fold:start:configure
configure: loading site script /Users/michael/github/bitcoin/depends/x86_64-unknown-linux-gnu/share/config.site
configure: loading cache config.cache
checking build system type... x86_64-pc-linux-gnu
checking host system type... x86_64-unknown-linux-gnu
checking for a BSD-compatible install... /usr/bin/install -c
checking whether build environment is sane... yes
checking for x86_64-unknown-linux-gnu-strip... no
checking for strip... strip
checking for a thread-safe mkdir -p... /bin/mkdir -p
checking for gawk... no
checking for mawk... mawk
checking whether make sets $(MAKE)... yes
checking whether make supports nested variables... yes
configure: error: source directory already configured; run "make distclean" there first
This file contains any messages produced by compilers while
running configure, to aid debugging if configure makes a mistake.

It was created by Bitcoin Core configure 0.18.99, which was
generated by GNU Autoconf 2.69.  Invocation command line was

  $ ../configure --cache-file=config.cache --disable-dependency-tracking --prefix=/Users/michael/github/bitcoin/depends/x86_64-unknown-linux-gnu --bindir=/Users/michael/github/bitcoin/out/x86_64-unknown-linux-gnu/bin --libdir=/Users/michael/github/bitcoin/out/x86_64-unknown-linux-gnu/lib

MarcoFalke commented at 12:45 PM on August 15, 2019: member

It says source directory already configured; run "make distclean" there first. Have you tried that?

MarcoFalke commented at 12:48 PM on August 15, 2019: member

Specifically cd /Users/michael/github/bitcoin && make distclean.

The ci is generally assuming the the folder is a fresh git clone. If needed, that can be changed later on, but better not part of this pull.

fanquake commented at 12:49 PM on August 15, 2019: member

It says source directory already configured; run "make distclean" there first. Have you tried that?

Shouldn't that all be handled by the script? If not, then some docs about why you can't run the tests twice in a row would be handy.

MarcoFalke commented at 12:59 PM on August 15, 2019: member

Done

MarcoFalke force-pushed on Aug 15, 2019

fanquake approved

fanquake commented at 1:21 PM on August 15, 2019: member

Thanks ACK fa9404d76d16c618dbd2c19973246900918f9e6c - Maybe @sipsorcery or @NicolasDorier might also want to test here, then we'd have all OS's covered?

MarcoFalke commented at 1:53 PM on August 15, 2019: member

I don't think docker on windows can run Ubuntu containers

DrahtBot commented at 3:06 PM on August 15, 2019: member

Needs rebase

DrahtBot added the label Needs rebase on Aug 15, 2019

ci: Rename .travis/ to ./ci/ fafe78f6ae

ci: Add retry

From: https://github.com/kadwanev/retry/tree/a1b1826bdb0a78189d5c70c858dc676f5133b1d7

fa0aac0f43

ci: Remove dependence on travis, use it as fallback env fa31bc35eb

ci: Use ./ci/ on non-travis host fa6cbdc3c9

ci: Add note that this assumes a fresh git clone faeacf3269

MarcoFalke force-pushed on Aug 15, 2019

MarcoFalke commented at 3:15 PM on August 15, 2019: member

Rebased without any changes due to overly sensitive merge conflict detection on GitHub. git range-diff should be empty: git range-diff bitcoin/master fa9404d76d faeacf3269

MarcoFalke merged this on Aug 15, 2019

MarcoFalke closed this on Aug 15, 2019

MarcoFalke deleted the branch on Aug 15, 2019

MarcoFalke referenced this in commit 367b023ae4 on Aug 15, 2019

sipsorcery commented at 7:16 PM on August 15, 2019: member

I don't think docker on windows can run Ubuntu containers

It can since https://docs.docker.com/docker-for-windows/wsl-tech-preview/

aaron@pcdodo:~$ uname -a
Linux pcdodo 4.19.57-microsoft-standard [#1](/bitcoin-bitcoin/1/) SMP Wed Jul 3 20:47:25 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux
aaron@pcdodo:~$ docker run ubuntu echo "I  love Windows"; lsb_release -a
I  love Windows
No LSB modules are available.
Distributor ID: Ubuntu
Description:    Ubuntu 18.04.2 LTS
Release:        18.04
Codename:       bionic

Actually it was already possible to use ubuntu docker containers on Windows but now it possible directly on top of the Windows Subsystem for Linux kernel.

sipsorcery commented at 7:21 PM on August 15, 2019: member

Thanks ACK fa9404d - Maybe @sipsorcery or @NicolasDorier might also want to test here, then we'd have all OS's covered? @fanquake I looked over the PR and successfully ran my appveyor build with the PR merged. Doesn't seem to be anything Windows related in this PR? Ony travis and shell scripts. Or did you testing the script changes with the WSL/Mingw builds on Windows?

fanquake commented at 1:02 AM on August 16, 2019: member

Thanks @sipsorcery. I also thought it was possible to run on Windows.

Or did you testing the script changes with the WSL/Mingw builds on Windows?

Yea I just meant checking out this PR, and testing/running the scripts, to make sure they were working in your Windows development setups.

laanwj removed the label Needs rebase on Oct 24, 2019

vijaydasmp referenced this in commit bbae812c5d on Oct 2, 2021

kittywhiskers referenced this in commit 68cd29c0ab on Dec 13, 2021

DrahtBot locked this on Dec 16, 2021