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

First attempt at CI Testing #204 #205

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

reactive-firewall
Copy link

First attempt at #204

  • tests run every time code is pushed to github (auto-tests)
  • tests show passing example (just a test that usage is output when the -h flag is passed)
  • tests shows failure case with error
  • tests check themselves (both passing and failing example)
  • tests are run in the cloud and thus can be configured to run in multiple environments independent of developer's system (caveat: tests on linux only in this PR)
  • this PR does not break nor alter non-testing code or function
  • maintainer can choose to make tests required for merges

🤔 ... Thoughts? Reviews? Thumbs up?

Copy link
Author

@reactive-firewall reactive-firewall left a comment

Choose a reason for hiding this comment

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

Inline comments on the logic of key parts of the code have been added inline as a review. 🤔 now if I can get @laurent22 to look at the code and give feedback ...

reviews (comments and suggestions) are MOST welcome!

echo "WARNING: No Cleanup"

##############################
# can more run setup code here
Copy link
Author

Choose a reason for hiding this comment

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

Basically this script is a NO-OP

Comment on lines +1 to +145
# CONSEQUENTIAL DAMAGES WHATSOEVER, INCLUDING, WITHOUT LIMITATION, DAMAGES
# FOR LOSS OF PROFITS, CORRUPTION OR LOSS OF DATA, FAILURE TO TRANSMIT OR
# RECEIVE ANY DATA OR INFORMATION, BUSINESS INTERRUPTION OR ANY OTHER
# COMMERCIAL DAMAGES OR LOSSES, ARISING OUT OF OR RELATED TO YOUR USE OR
# INABILITY TO USE THIS SHELL SCRIPT OR SERVICES OR ANY THIRD PARTY
# SOFTWARE OR APPLICATIONS IN CONJUNCTION WITH THIS SHELL SCRIPT OR
# SERVICES, HOWEVER CAUSED, REGARDLESS OF THE THEORY OF LIABILITY (CONTRACT,
# TORT OR OTHERWISE) AND EVEN IF THE AUTHOR HAS BEEN ADVISED OF THE
# POSSIBILITY OF SUCH DAMAGES. SOME JURISDICTIONS DO NOT ALLOW THE EXCLUSION
# OR LIMITATION OF LIABILITY FOR PERSONAL INJURY, OR OF INCIDENTAL OR
# CONSEQUENTIAL DAMAGES, SO THIS LIMITATION MAY NOT APPLY TO YOU. In no event
# shall THE AUTHOR's total liability to you for all damages (other than as may
# be required by applicable law in cases involving personal injury) exceed
# the amount of five dollars ($5.00). The foregoing limitations will apply
# even if the above stated remedy fails of its essential purpose.
################################################################################

# bootstrap checks (pre pre-checks)
EXIT_CODE=0

test -x `command -v uname` || exit 1 ;
test -x `command -v git` || exit 1 ;

if [[ ( $(uname -s) = "Darwin" ) ]] ; then
export CI_OS_NAME="osx" ;
else
export CI_OS_NAME="linux" ;
fi


# if locking was needed:
####
# ulimit -t 600
# PATH="/bin:/sbin:/usr/sbin:/usr/bin"
# umask 137
#
# LOCK_FILE="/tmp/pre_test_script_lock"
#
# if [[ -f ${LOCK_FILE} ]] ; then
# exit 0 ;
# fi
#
# trap 'rm -f ${LOCK_FILE} 2>/dev/null || true ; wait ; exit 1 ;' SIGABORT
# trap 'rm -f ${LOCK_FILE} 2>/dev/null || true ; wait ; exit 1 ;' SIGHUP
# trap 'rm -f ${LOCK_FILE} 2>/dev/null || true ; wait ; exit 1 ;' SIGTERM
# trap 'rm -f ${LOCK_FILE} 2>/dev/null || true ; wait ; exit 1 ;' SIGQUIT
# trap 'rm -f ${LOCK_FILE} 2>/dev/null || true ; wait ; exit 1 ;' SIGINT
# trap 'rm -f ${LOCK_FILE} 2>/dev/null || true ; wait ; exit 0 ;' EXIT
#
# touch ${LOCK_FILE} 2>/dev/null || exit 0 ;

# THIS IS THE ACTUAL WORK:

# use this for codecov.io if enabled
#export CODECOV_TOKEN="" ;
# use for a github token if enabled (USE HARDCODE AT OWN RISK)
#export GITHUB_TOKEN="" ;
export CI=true ; # we are running CI
export CONTINUOUS_INTEGRATION=true ;
# if you run on-prem
#export GITLAB_CI=true ;
# get the repository name
export CI_REPO=$(basename `git rev-parse --show-toplevel`) ;
export CI_BRANCH=$(git symbolic-ref --short HEAD ) ;
export GIT_BRANCH=$(git symbolic-ref --short HEAD ) ;
export BRANCH_NAME=$(git symbolic-ref --short HEAD) ;
export SOURCE_BRANCH=$(git symbolic-ref --short HEAD ) ;
export CF_BRANCH=$(git symbolic-ref --short HEAD ) ;
export CIRCLE_BRANCH=$(git symbolic-ref --short HEAD ) ;
export APPVEYOR_REPO_BRANCH=$(git symbolic-ref --short HEAD ) ;
export TEAMCITY_BUILD_BRANCH=$(git symbolic-ref --short HEAD ) ;
export VCS_BRANCH_NAME=$(git symbolic-ref --short HEAD ) ;
export TRAVIS_BRANCH=$(git symbolic-ref --short HEAD ) ;
# Next line commented as this can mess with codecov if in use
# export WERCKER_GIT_BRANCH=$(git symbolic-ref --short HEAD ) ;
export TRAVIS_OS_NAME="${TRAVIS_OS_NAME:-${CI_OS_NAME}}" ;
export TRAVIS_PYTHON_VERSION=$($(command -v python) --version 2>&1 3>&1 | head -n 1 | grep -oE "[0-9]+.[0-9]+" | head -n 1 ) ;
export TRAVIS_COMMIT=$(git rev-parse --verify HEAD)
export CI_BUILD_ID=$(git rev-list --all --count)
export CI_BUILD_REF=$(git rev-parse --verify HEAD)
export CI_JOB_ID=$(date '+%y%m%d%H%M%S')
export CI_COMMIT_NAME=$(git symbolic-ref --short HEAD ) ;
# used by some tools to link back to this build's log
#export CI_BUILD_URL="https://localhost:443/codecov/fake/ci/build/${CI_BUILD_ID}/${CI_JOB_ID}"
export VCS_COMMIT_ID=$(git rev-parse --verify HEAD)
export GITHUB_USER=$(git config --get user.name)
export GITHUB_REPO=$(basename `git rev-parse --show-toplevel`) ;
export GITHUB_COMMENTS_URL="https://api.github.com/repos/${GITHUB_USER}/${GITHUB_REPO}/commits/${CI_BUILD_REF}/comments"

##############################
# can more run setup code here
##############################
# ideas: pull down resources need for tests like docker images or test tools

# if locking is used: clean up here
####
# rm -f ${LOCK_FILE} >/dev/null || true ; wait ;

# goodbye
exit ${EXIT_CODE:-255} ;
Copy link
Author

Choose a reason for hiding this comment

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

This file is sparse but setup some key env for use later.

Comment on lines +82 to +97
LOCK_FILE="/tmp/CI_test_script_lock"

if [[ -f ${LOCK_FILE} ]] ; then
echo "Lock found! ABORT"
exit ${EXIT_CODE:-0} ;
fi
#
trap 'rm -f ${LOCK_FILE} 2>/dev/null || true ; wait ; exit 1 ;' SIGABRT
trap 'rm -f ${LOCK_FILE} 2>/dev/null || true ; wait ; exit 1 ;' SIGHUP
trap 'rm -f ${LOCK_FILE} 2>/dev/null || true ; wait ; exit 1 ;' SIGTERM
trap 'rm -f ${LOCK_FILE} 2>/dev/null || true ; wait ; exit 1 ;' SIGQUIT
trap 'rm -f ${LOCK_FILE} 2>/dev/null || true ; wait ; exit 1 ;' SIGINT
trap 'rm -f ${LOCK_FILE} 2>/dev/null || true ; wait ; exit 1 ;' SIGKILL
trap 'rm -f ${LOCK_FILE} 2>/dev/null || true ; wait ; exit ${EXIT_CODE:-0} ;' EXIT
#
touch ${LOCK_FILE} 2>/dev/null || exit 0 ;
Copy link
Author

Choose a reason for hiding this comment

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

Locking prevents multiple test runs from interfering with each-other in the future.

Comment on lines +101 to +105
export CI_TOOLS_BIN=$(dirname "${0}") ;
#TODO: make this more absolute
export CI_TEST_ROOT=$(dirname ${PWD:-./}/.) ;

${CI_TOOLS_BIN}/pretests.sh
Copy link
Author

Choose a reason for hiding this comment

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

Setup script is used here.

Comment on lines +107 to +115
function fn_test_pass() {
echo "... Passed" ;
return 0 ;
}

function fn_test_fail() {
echo "... FAILED" ;
return 1 ;
} 1>&2
Copy link
Author

Choose a reason for hiding this comment

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

By using functions for results test log results can be more consistent.

Comment on lines +149 to +150
### KEEP THIS LINE ###
${CI_TOOLS_BIN}/posttests.sh
Copy link
Author

Choose a reason for hiding this comment

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

This is where the cleanup script is called. caveat: it does nothing at the moment.

echo "STYLE CHECK"
export CI_TEST_ROOT=$(dirname ${PWD:-./}/.) ;

shellcheck "${CI_TEST_ROOT}/../../*.sh" || EXIT_CODE=1 ;
Copy link
Author

Choose a reason for hiding this comment

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

🔎 this line is the actual check

Comment on lines +6 to +19
META:

runs-on: ubuntu-latest

steps:
- uses: actions/checkout@v2
- name: Check Pre Test Setup Script
run: test -x ./.github/bin/pretests.sh ;
- name: Check Test Scripts
run: test -x ./.github/bin/runtests.sh ;
- name: Check Linter Scripts
run: test -x ./.github/bin/styletests.sh ;
- name: Check Post Test Scripts
run: test -x ./.github/bin/posttests.sh ;
Copy link
Author

Choose a reason for hiding this comment

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

This github workflow checks that the tests can be run. i.e. self-testing.

Comment on lines +21 to +32
MATS:

runs-on: ubuntu-latest

steps:
- uses: actions/checkout@v2
- name: Pre Test Setup
run: ./.github/bin/pretests.sh || true ;
- name: Run Tests
run: ./.github/bin/runtests.sh ;
- name: Post Test Clean
run: ./.github/bin/posttests.sh || true ;
Copy link
Author

Choose a reason for hiding this comment

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

This is the Minimal Acceptance Testing Suite. Every future change should pass MATS, or be fixed before merge. NON-Minimal Extra tests can be added like this as needed.

Comment on lines +34 to +45
STYLE:

runs-on: ubuntu-latest

steps:
- uses: actions/checkout@v2
- name: Pre Test Setup
run: ./.github/bin/pretests.sh || true ;
- name: Run Tests
run: ./.github/bin/styletests.sh || true ;
- name: Post Test Clean
run: ./.github/bin/posttests.sh || true ;
Copy link
Author

Choose a reason for hiding this comment

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

Reviewing code commits is hard: this set of tests checks for a LOT of code style issues, leading to more focus on function in reviews and less focus on syntax by us humans. Let the computers crunch the numbers.

@fthiery
Copy link

fthiery commented Apr 29, 2020

This is extremely needed IMO. Not a reason not to try, but some stuff is impossible to simulate unfortunately (e.g. #64)

Thanks for working on that

@reactive-firewall
Copy link
Author

@fthiery

but some stuff is impossible to simulate unfortunately (e.g. #64)

TL;DR I think you're missing an important "easily" between "to" and "simulate. With that assumption it might be worth pointing out that this (#64) kind of use-case would best be tested by mocking the issue. By mocking the failed output of rsync we can skip the long time of waiting on rsync while testing this case. This test would be something like replacing the rsync command with a mocked rsync script that just outputs the lost error behavior without the data, thus separating out the fail case so it may be used to test the handling function.

caveat: however I could be totally wrong and you may mean something else.

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.

None yet

2 participants