From 54d350001f3076cd0e7d2de5b6672dddd778a72e Mon Sep 17 00:00:00 2001 From: James Lamb Date: Sat, 11 Jan 2025 21:56:03 -0600 Subject: [PATCH] [ci] add linting and autoformatting for shell scripts (#227) --- .ci/install.sh | 8 +- .ci/report_to_covr.sh | 4 +- .ci/setup.sh | 24 +- .ci/test.sh | 14 +- .pre-commit-config.yaml | 22 ++ cleanup_local.sh | 6 +- cran-comments.md | 4 +- r-pkg/R/es_search.R | 2 +- r-pkg/tests/testthat/test-integration.R | 4 +- setup_local.sh | 282 +++++++++++++----------- 10 files changed, 206 insertions(+), 164 deletions(-) create mode 100644 .pre-commit-config.yaml diff --git a/.ci/install.sh b/.ci/install.sh index 1b0c32e..d9a5a22 100755 --- a/.ci/install.sh +++ b/.ci/install.sh @@ -1,10 +1,10 @@ #!/bin/bash # failure is a natural part of life -set -e +set -e -u -o pipefail if [[ "$TASK" == "rpkg" ]]; then - R CMD INSTALL \ - --clean \ - $(pwd)/r-pkg + R CMD INSTALL \ + --clean \ + ./r-pkg fi diff --git a/.ci/report_to_covr.sh b/.ci/report_to_covr.sh index f5138f6..77cb503 100755 --- a/.ci/report_to_covr.sh +++ b/.ci/report_to_covr.sh @@ -1,10 +1,10 @@ #!/bin/bash # failure is a natural part of life -set -e +set -e -u -o pipefail if [[ "$TASK" == "rpkg" ]]; then - Rscript -e " \ + Rscript -e " \ Sys.setenv(NOT_CRAN = 'true'); \ covr::codecov('r-pkg/') \ " diff --git a/.ci/setup.sh b/.ci/setup.sh index 5365f29..42e8f05 100755 --- a/.ci/setup.sh +++ b/.ci/setup.sh @@ -1,10 +1,11 @@ +#!/bin/bash + # failure is a natural part of life -set -e +set -e -u -o pipefail # If language: r, # install these testing packages we need -if [[ "$TASK" == "rpkg" ]]; -then +if [[ "$TASK" == "rpkg" ]]; then # `devscripts` is required for 'checkbashisms' (https://github.com/r-lib/actions/issues/111) sudo apt-get update @@ -12,15 +13,14 @@ then --no-install-recommends \ -y \ --allow-downgrades \ - libcurl4-openssl-dev \ - curl \ - devscripts \ - texinfo \ - texlive-latex-recommended \ - texlive-fonts-recommended \ - texlive-fonts-extra \ - qpdf \ - || exit -1 + libcurl4-openssl-dev \ + curl \ + devscripts \ + texinfo \ + texlive-latex-recommended \ + texlive-fonts-recommended \ + texlive-fonts-extra \ + qpdf Rscript -e "install.packages(c('assertthat', 'covr', 'data.table', 'futile.logger', 'httr', 'jsonlite', 'knitr', 'lintr', 'purrr', 'rmarkdown', 'stringr', 'testthat', 'uuid'), repos = 'https://cran.r-project.org', Ncpus = parallel::detectCores())" cp test-data/* r-pkg/inst/testdata/ diff --git a/.ci/test.sh b/.ci/test.sh index fbfe8f7..1ec51ff 100755 --- a/.ci/test.sh +++ b/.ci/test.sh @@ -1,13 +1,13 @@ #!/bin/bash # failure is a natural part of life -set -e +set -e -u -o pipefail if [[ "$TASK" == "rpkg" ]]; then - Rscript .ci/lint_r_code.R $(pwd) - R CMD build $(pwd)/r-pkg - export _R_CHECK_CRAN_INCOMING_=false - R CMD check \ - --as-cran \ - *.tar.gz + Rscript .ci/lint_r_code.R "$(pwd)" + R CMD build ./r-pkg + export _R_CHECK_CRAN_INCOMING_=false + R CMD check \ + --as-cran \ + ./*.tar.gz fi diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml new file mode 100644 index 0000000..153a4e0 --- /dev/null +++ b/.pre-commit-config.yaml @@ -0,0 +1,22 @@ +--- +exclude: | + (?x)^( + test-data/.* + )$ +repos: + - repo: https://github.com/maxwinterstein/shfmt-py + rev: v3.7.0.1 + hooks: + - id: shfmt + args: ["--indent=4", "--space-redirects", "--write"] + - repo: https://github.com/shellcheck-py/shellcheck-py + rev: v0.10.0.1 + hooks: + - id: shellcheck + args: ["--exclude=SC2002"] + - repo: https://github.com/codespell-project/codespell + rev: v2.3.0 + hooks: + - id: codespell + # additional_dependencies: [tomli] + # args: ["--toml", "pyproject.toml"] diff --git a/cleanup_local.sh b/cleanup_local.sh index 486c3e7..90e9eb4 100755 --- a/cleanup_local.sh +++ b/cleanup_local.sh @@ -1,13 +1,13 @@ #!/bin/bash -set -e +set -e -u -o pipefail # Remove testing directory echo "removing testing directory" -rm -r $(pwd)/sandbox +rm -r ./sandbox # Kill the running container echo "killing running container" -docker kill $(docker ps -ql) +docker kill "$(docker ps -ql)" echo "done cleaning up test environment" diff --git a/cran-comments.md b/cran-comments.md index eb3434a..9806f1c 100644 --- a/cran-comments.md +++ b/cran-comments.md @@ -14,7 +14,7 @@ * One NOTE from `checking CRAN incoming feasibility ...` can be safely ignored since it's a note that notifies CRAN that this is a new maintainer/submission. ### CRAN Response -* Automatic checking upon CRAN submission yielded two notes. One was the "incoming feasbility..." item we mentioned above, which is not an issue. +* Automatic checking upon CRAN submission yielded two notes. One was the "incoming feasibility..." item we mentioned above, which is not an issue. * The other note said that `Author field differs from that derived from Authors@R`. This did not arise when running `R CMD check --as-cran` locally, but it looks like "fnd" is not a supported tag for an author. Removed that tag. ## v0.0.2 - Submission 2 - (July 17, 2017) @@ -127,7 +127,7 @@ In this submission, we changed maintainer from `james.lamb@uptake.com` to `jayla ## v0.4.0 - Submission 1 - (September 11, 2019) ### `R CMD check` results -* No isses +* No issues ### CRAN Response * No issues. v0.4.0 released to CRAN! diff --git a/r-pkg/R/es_search.R b/r-pkg/R/es_search.R index 3498d89..ff144ed 100644 --- a/r-pkg/R/es_search.R +++ b/r-pkg/R/es_search.R @@ -641,7 +641,7 @@ es_search <- function(es_host log_info(sprintf("uptasticsearch thinks you are running Elasticsearch %s", version)) # Parse out just the major version. We can adjust this if we find - # API differences that occured at the minor version level + # API differences that occurred at the minor version level major_version <- .major_version(version) return(major_version) } diff --git a/r-pkg/tests/testthat/test-integration.R b/r-pkg/tests/testthat/test-integration.R index 5e1cda7..35c75fd 100644 --- a/r-pkg/tests/testthat/test-integration.R +++ b/r-pkg/tests/testthat/test-integration.R @@ -166,7 +166,7 @@ futile.logger::flog.threshold(0) , ignore.order = TRUE ) - # ther stuff we might as well test + # the stuff we might as well test expect_true(data.table::is.data.table(outDT)) expect_true(is.numeric(outDT[, doc_count])) expect_true(is.character(outDT[, name_i_picked])) @@ -200,7 +200,7 @@ futile.logger::flog.threshold(0) # Decided to check that it's coercible to an integer instead of # hard-coding known Elasticsearch versions so this test won't require - # attention or break builds if/when Elasticsearch 7 or whatever the next major verison + # attention or break builds if/when Elasticsearch 7 or whatever the next major version # is comes out expect_true(!is.na(as.integer(ver)), info = paste0("returned version: ", ver)) }) diff --git a/setup_local.sh b/setup_local.sh index 6ee1bb1..94da383 100755 --- a/setup_local.sh +++ b/setup_local.sh @@ -16,126 +16,146 @@ echo "Starting up Elasticsearch..." case "${ES_VERSION}" in -1.0.3) docker run -d -p 9200:9200 barnybug/elasticsearch:1.0.3 - MAPPING_FILE=$(pwd)/test-data/legacy_shakespeare_mapping.json - ;; -1.7.6) docker run -d -p 9200:9200 elasticsearch:1.7.6 - MAPPING_FILE=$(pwd)/test-data/legacy_shakespeare_mapping.json - ;; -2.4.6) docker run -d -p 9200:9200 elasticsearch:2.4.6 - MAPPING_FILE=$(pwd)/test-data/legacy_shakespeare_mapping.json - ;; -5.6.16) docker run -d -p 9200:9200 \ - -e "xpack.security.enabled=false" \ - docker.elastic.co/elasticsearch/elasticsearch:5.6.16 - MAPPING_FILE=$(pwd)/test-data/es5_shakespeare_mapping.json - ;; -6.0.1) docker run -d -p 9200:9200 \ - -e "discovery.type=single-node" \ - -e "xpack.security.enabled=false" \ - docker.elastic.co/elasticsearch/elasticsearch:6.0.1 - MAPPING_FILE=$(pwd)/test-data/es6_shakespeare_mapping.json - ;; -6.8.15) docker run -d -p 9200:9200 \ - -e "discovery.type=single-node" \ - -e "xpack.security.enabled=false" \ - docker.elastic.co/elasticsearch/elasticsearch:6.8.11 - MAPPING_FILE=$(pwd)/test-data/es6_shakespeare_mapping.json - ;; -7.0.1) docker run -d -p 9200:9200 \ - -e "discovery.type=single-node" \ - -e "xpack.security.enabled=false" \ - docker.elastic.co/elasticsearch/elasticsearch:7.0.1 - MAPPING_FILE=$(pwd)/test-data/es7_shakespeare_mapping.json - SAMPLE_DATA_FILE=$(pwd)/test-data/sample_es7.json - ;; -7.1.1) docker run -d -p 9200:9200 \ - -e "discovery.type=single-node" \ - -e "xpack.security.enabled=false" \ - docker.elastic.co/elasticsearch/elasticsearch:7.1.1 - MAPPING_FILE=$(pwd)/test-data/es7_shakespeare_mapping.json - SAMPLE_DATA_FILE=$(pwd)/test-data/sample_es7.json - ;; -7.2.1) docker run -d -p 9200:9200 \ - -e "discovery.type=single-node" \ - -e "xpack.security.enabled=false" \ - docker.elastic.co/elasticsearch/elasticsearch:7.2.1 - MAPPING_FILE=$(pwd)/test-data/es7_shakespeare_mapping.json - SAMPLE_DATA_FILE=$(pwd)/test-data/sample_es7.json - ;; -7.3.2) docker run -d -p 9200:9200 \ - -e "discovery.type=single-node" \ - -e "xpack.security.enabled=false" \ - docker.elastic.co/elasticsearch/elasticsearch:7.3.2 - MAPPING_FILE=$(pwd)/test-data/es7_shakespeare_mapping.json - SAMPLE_DATA_FILE=$(pwd)/test-data/sample_es7.json - ;; -7.4.2) docker run -d -p 9200:9200 \ - -e "discovery.type=single-node" \ - -e "xpack.security.enabled=false" \ - docker.elastic.co/elasticsearch/elasticsearch:7.4.2 - MAPPING_FILE=$(pwd)/test-data/es7_shakespeare_mapping.json - SAMPLE_DATA_FILE=$(pwd)/test-data/sample_es7.json - ;; -7.5.2) docker run -d -p 9200:9200 \ - -e "discovery.type=single-node" \ - -e "xpack.security.enabled=false" \ - docker.elastic.co/elasticsearch/elasticsearch:7.5.2 - MAPPING_FILE=$(pwd)/test-data/es7_shakespeare_mapping.json - SAMPLE_DATA_FILE=$(pwd)/test-data/sample_es7.json - ;; -7.6.2) docker run -d -p 9200:9200 \ - -e "discovery.type=single-node" \ - -e "xpack.security.enabled=false" \ - docker.elastic.co/elasticsearch/elasticsearch:7.6.2 - MAPPING_FILE=$(pwd)/test-data/es7_shakespeare_mapping.json - SAMPLE_DATA_FILE=$(pwd)/test-data/sample_es7.json - ;; -7.7.1) docker run -d -p 9200:9200 \ - -e "discovery.type=single-node" \ - -e "xpack.security.enabled=false" \ - docker.elastic.co/elasticsearch/elasticsearch:7.7.1 - MAPPING_FILE=$(pwd)/test-data/es7_shakespeare_mapping.json - SAMPLE_DATA_FILE=$(pwd)/test-data/sample_es7.json - ;; -7.8.1) docker run -d -p 9200:9200 \ - -e "discovery.type=single-node" \ - -e "xpack.security.enabled=false" \ - docker.elastic.co/elasticsearch/elasticsearch:7.8.1 - MAPPING_FILE=$(pwd)/test-data/es7_shakespeare_mapping.json - SAMPLE_DATA_FILE=$(pwd)/test-data/sample_es7.json - ;; -7.9.3) docker run -d -p 9200:9200 \ - -e "discovery.type=single-node" \ - -e "xpack.security.enabled=false" \ - docker.elastic.co/elasticsearch/elasticsearch:7.9.3 - MAPPING_FILE=$(pwd)/test-data/es7_shakespeare_mapping.json - SAMPLE_DATA_FILE=$(pwd)/test-data/sample_es7.json - ;; -7.10.2) docker run -d -p 9200:9200 \ - -e "discovery.type=single-node" \ - -e "xpack.security.enabled=false" \ - docker.elastic.co/elasticsearch/elasticsearch:7.10.2 - MAPPING_FILE=$(pwd)/test-data/es7_shakespeare_mapping.json - SAMPLE_DATA_FILE=$(pwd)/test-data/sample_es7.json - ;; -7.11.2) docker run -d -p 9200:9200 \ - -e "discovery.type=single-node" \ - -e "xpack.security.enabled=false" \ - docker.elastic.co/elasticsearch/elasticsearch:7.11.2 - MAPPING_FILE=$(pwd)/test-data/es7_shakespeare_mapping.json - SAMPLE_DATA_FILE=$(pwd)/test-data/sample_es7.json - ;; -7.12.1) docker run -d -p 9200:9200 \ - -e "discovery.type=single-node" \ - -e "xpack.security.enabled=false" \ - docker.elastic.co/elasticsearch/elasticsearch:7.12.1 - MAPPING_FILE=$(pwd)/test-data/es7_shakespeare_mapping.json - SAMPLE_DATA_FILE=$(pwd)/test-data/sample_es7.json - ;; -*) echo "Did not recognize version ${ES_VERSION}. Not starting Elasticsearch" - exit 1 - ;; +1.0.3) + docker run -d -p 9200:9200 barnybug/elasticsearch:1.0.3 + MAPPING_FILE=$(pwd)/test-data/legacy_shakespeare_mapping.json + ;; +1.7.6) + docker run -d -p 9200:9200 elasticsearch:1.7.6 + MAPPING_FILE=$(pwd)/test-data/legacy_shakespeare_mapping.json + ;; +2.4.6) + docker run -d -p 9200:9200 elasticsearch:2.4.6 + MAPPING_FILE=$(pwd)/test-data/legacy_shakespeare_mapping.json + ;; +5.6.16) + docker run -d -p 9200:9200 \ + -e "xpack.security.enabled=false" \ + docker.elastic.co/elasticsearch/elasticsearch:5.6.16 + MAPPING_FILE=$(pwd)/test-data/es5_shakespeare_mapping.json + ;; +6.0.1) + docker run -d -p 9200:9200 \ + -e "discovery.type=single-node" \ + -e "xpack.security.enabled=false" \ + docker.elastic.co/elasticsearch/elasticsearch:6.0.1 + MAPPING_FILE=$(pwd)/test-data/es6_shakespeare_mapping.json + ;; +6.8.15) + docker run -d -p 9200:9200 \ + -e "discovery.type=single-node" \ + -e "xpack.security.enabled=false" \ + docker.elastic.co/elasticsearch/elasticsearch:6.8.11 + MAPPING_FILE=$(pwd)/test-data/es6_shakespeare_mapping.json + ;; +7.0.1) + docker run -d -p 9200:9200 \ + -e "discovery.type=single-node" \ + -e "xpack.security.enabled=false" \ + docker.elastic.co/elasticsearch/elasticsearch:7.0.1 + MAPPING_FILE=$(pwd)/test-data/es7_shakespeare_mapping.json + SAMPLE_DATA_FILE=$(pwd)/test-data/sample_es7.json + ;; +7.1.1) + docker run -d -p 9200:9200 \ + -e "discovery.type=single-node" \ + -e "xpack.security.enabled=false" \ + docker.elastic.co/elasticsearch/elasticsearch:7.1.1 + MAPPING_FILE=$(pwd)/test-data/es7_shakespeare_mapping.json + SAMPLE_DATA_FILE=$(pwd)/test-data/sample_es7.json + ;; +7.2.1) + docker run -d -p 9200:9200 \ + -e "discovery.type=single-node" \ + -e "xpack.security.enabled=false" \ + docker.elastic.co/elasticsearch/elasticsearch:7.2.1 + MAPPING_FILE=$(pwd)/test-data/es7_shakespeare_mapping.json + SAMPLE_DATA_FILE=$(pwd)/test-data/sample_es7.json + ;; +7.3.2) + docker run -d -p 9200:9200 \ + -e "discovery.type=single-node" \ + -e "xpack.security.enabled=false" \ + docker.elastic.co/elasticsearch/elasticsearch:7.3.2 + MAPPING_FILE=$(pwd)/test-data/es7_shakespeare_mapping.json + SAMPLE_DATA_FILE=$(pwd)/test-data/sample_es7.json + ;; +7.4.2) + docker run -d -p 9200:9200 \ + -e "discovery.type=single-node" \ + -e "xpack.security.enabled=false" \ + docker.elastic.co/elasticsearch/elasticsearch:7.4.2 + MAPPING_FILE=$(pwd)/test-data/es7_shakespeare_mapping.json + SAMPLE_DATA_FILE=$(pwd)/test-data/sample_es7.json + ;; +7.5.2) + docker run -d -p 9200:9200 \ + -e "discovery.type=single-node" \ + -e "xpack.security.enabled=false" \ + docker.elastic.co/elasticsearch/elasticsearch:7.5.2 + MAPPING_FILE=$(pwd)/test-data/es7_shakespeare_mapping.json + SAMPLE_DATA_FILE=$(pwd)/test-data/sample_es7.json + ;; +7.6.2) + docker run -d -p 9200:9200 \ + -e "discovery.type=single-node" \ + -e "xpack.security.enabled=false" \ + docker.elastic.co/elasticsearch/elasticsearch:7.6.2 + MAPPING_FILE=$(pwd)/test-data/es7_shakespeare_mapping.json + SAMPLE_DATA_FILE=$(pwd)/test-data/sample_es7.json + ;; +7.7.1) + docker run -d -p 9200:9200 \ + -e "discovery.type=single-node" \ + -e "xpack.security.enabled=false" \ + docker.elastic.co/elasticsearch/elasticsearch:7.7.1 + MAPPING_FILE=$(pwd)/test-data/es7_shakespeare_mapping.json + SAMPLE_DATA_FILE=$(pwd)/test-data/sample_es7.json + ;; +7.8.1) + docker run -d -p 9200:9200 \ + -e "discovery.type=single-node" \ + -e "xpack.security.enabled=false" \ + docker.elastic.co/elasticsearch/elasticsearch:7.8.1 + MAPPING_FILE=$(pwd)/test-data/es7_shakespeare_mapping.json + SAMPLE_DATA_FILE=$(pwd)/test-data/sample_es7.json + ;; +7.9.3) + docker run -d -p 9200:9200 \ + -e "discovery.type=single-node" \ + -e "xpack.security.enabled=false" \ + docker.elastic.co/elasticsearch/elasticsearch:7.9.3 + MAPPING_FILE=$(pwd)/test-data/es7_shakespeare_mapping.json + SAMPLE_DATA_FILE=$(pwd)/test-data/sample_es7.json + ;; +7.10.2) + docker run -d -p 9200:9200 \ + -e "discovery.type=single-node" \ + -e "xpack.security.enabled=false" \ + docker.elastic.co/elasticsearch/elasticsearch:7.10.2 + MAPPING_FILE=$(pwd)/test-data/es7_shakespeare_mapping.json + SAMPLE_DATA_FILE=$(pwd)/test-data/sample_es7.json + ;; +7.11.2) + docker run -d -p 9200:9200 \ + -e "discovery.type=single-node" \ + -e "xpack.security.enabled=false" \ + docker.elastic.co/elasticsearch/elasticsearch:7.11.2 + MAPPING_FILE=$(pwd)/test-data/es7_shakespeare_mapping.json + SAMPLE_DATA_FILE=$(pwd)/test-data/sample_es7.json + ;; +7.12.1) + docker run -d -p 9200:9200 \ + -e "discovery.type=single-node" \ + -e "xpack.security.enabled=false" \ + docker.elastic.co/elasticsearch/elasticsearch:7.12.1 + MAPPING_FILE=$(pwd)/test-data/es7_shakespeare_mapping.json + SAMPLE_DATA_FILE=$(pwd)/test-data/sample_es7.json + ;; +*) + echo "Did not recognize version ${ES_VERSION}. Not starting Elasticsearch" + exit 1 + ;; esac echo "Elasticsearch v${ES_VERSION} is now running at http://${ES_HOST}:9200" @@ -143,30 +163,30 @@ echo "Elasticsearch v${ES_VERSION} is now running at http://${ES_HOST}:9200" echo "Setting up local testing environment" # Creating testing directory -mkdir -p ${TESTDIR} +mkdir -p "${TESTDIR}" # Get data -cp ${MAPPING_FILE} ${TESTDIR}/shakespeare_mapping.json -cp ${SAMPLE_DATA_FILE} ${TESTDIR}/sample.json -cd ${TESTDIR} +cp "${MAPPING_FILE}" "${TESTDIR}/shakespeare_mapping.json" +cp "${SAMPLE_DATA_FILE}" "${TESTDIR}/sample.json" +cd "${TESTDIR}" # give the cluster a chance sleep 30 # Create shakespeare index and shakespeare mapping curl -X PUT "http://${ES_HOST}:9200/shakespeare" \ - -H 'Content-Type: application/json' \ - -d @shakespeare_mapping.json + -H 'Content-Type: application/json' \ + -d @shakespeare_mapping.json # Upload data curl -X POST "http://${ES_HOST}:9200/shakespeare/_bulk" \ - -H 'Content-Type: application/json' \ - --data-binary @sample.json + -H 'Content-Type: application/json' \ + --data-binary @sample.json # Add an intentionally empty index curl -X PUT "http://${ES_HOST}:9200/empty_index" \ - -H 'Content-Type: application/json' \ - -d @shakespeare_mapping.json + -H 'Content-Type: application/json' \ + -d @shakespeare_mapping.json # Refresh all indices curl -X POST "http://${ES_HOST}:9200/_refresh" @@ -174,7 +194,7 @@ curl -X POST "http://${ES_HOST}:9200/_refresh" # Check that we got something curl -X GET "http://${ES_HOST}:9200/shakespeare/_search?size=1" -cd ${WDIR} +cd "${WDIR}" echo "" echo "Your local environment is ready."