From 5946a12ab98196bc294412c26672235350663128 Mon Sep 17 00:00:00 2001 From: reesporte Date: Thu, 24 Mar 2022 15:20:21 -0500 Subject: [PATCH] fix viewTimePart to account for bad strings this stems from https://molecula.atlassian.net/browse/SUP-194 where the string "standard" was being passed to viewTimePart, which output "standard" as the result. this is not a valid time string and was causing confusing errors. now it simply doesn't do that this commit also adds regression testing framework and a regression test for fb-1287 --- .gitlab/.gitlab-ci.yml | 2 +- ...st_FB-1270_repro.sh => bug_repro_tests.sh} | 12 ++-- qa/scripts/utilCluster.sh | 1 + qa/testcases/bug-repros/README | 17 ++++++ qa/testcases/bug-repros/fb-1287-datagen.yaml | 57 +++++++++++++++++++ qa/testcases/bug-repros/fb-1287-test.sh | 10 ++++ .../test.sh => bug-repros/fb1270-test.sh} | 0 qa/testcases/bug-repros/run-all.sh | 8 +++ time.go | 6 +- time_internal_test.go | 12 ++++ 10 files changed, 117 insertions(+), 8 deletions(-) rename qa/scripts/{test_FB-1270_repro.sh => bug_repro_tests.sh} (76%) create mode 100644 qa/testcases/bug-repros/README create mode 100644 qa/testcases/bug-repros/fb-1287-datagen.yaml create mode 100755 qa/testcases/bug-repros/fb-1287-test.sh rename qa/testcases/{FB-1270_repro/test.sh => bug-repros/fb1270-test.sh} (100%) create mode 100755 qa/testcases/bug-repros/run-all.sh diff --git a/.gitlab/.gitlab-ci.yml b/.gitlab/.gitlab-ci.yml index 3d18236af..6b9de74c0 100644 --- a/.gitlab/.gitlab-ci.yml +++ b/.gitlab/.gitlab-ci.yml @@ -387,7 +387,7 @@ smoke test: script: - ./qa/scripts/setupSmokeTest.sh - ./qa/scripts/testSmokeTest.sh - - ./qa/scripts/test_FB-1270_repro.sh + - ./qa/scripts/bug_repro_tests.sh after_script: - ./qa/scripts/teardownSmokeTest.sh needs: diff --git a/qa/scripts/test_FB-1270_repro.sh b/qa/scripts/bug_repro_tests.sh similarity index 76% rename from qa/scripts/test_FB-1270_repro.sh rename to qa/scripts/bug_repro_tests.sh index bdab77cc4..f4f868e32 100755 --- a/qa/scripts/test_FB-1270_repro.sh +++ b/qa/scripts/bug_repro_tests.sh @@ -17,7 +17,7 @@ echo "using DATANODE0 ${DATANODE0}" ssh -A -i ~/.ssh/gitlab-featurebase-ci.pem -o "StrictHostKeyChecking no" ec2-user@${INGESTNODE0} "sudo yum -y install librdkafka" echo "Copying tests to remote" -scp -r -i ~/.ssh/gitlab-featurebase-ci.pem ./qa/testcases/FB-1270_repro/test.sh ec2-user@${INGESTNODE0}:/data +scp -r -i ~/.ssh/gitlab-featurebase-ci.pem ./qa/testcases/bug-repros/ ec2-user@${INGESTNODE0}:/data scp -r -i ~/.ssh/gitlab-featurebase-ci.pem ./datagen_linux_arm64 ec2-user@${INGESTNODE0}:/data if (( $? != 0 )) then @@ -25,17 +25,17 @@ then exit 1 fi -# run 1270 repro -echo "Running smoke test..." -ssh -A -i ~/.ssh/gitlab-featurebase-ci.pem -o "StrictHostKeyChecking no" ec2-user@${INGESTNODE0} "cd /data/; ./test.sh ${DATANODE0}:10101" +# run all repros +echo "Running smoke tests..." +ssh -A -i ~/.ssh/gitlab-featurebase-ci.pem -o "StrictHostKeyChecking no" ec2-user@${INGESTNODE0} "cd /data/bug-repros; ./run-all.sh ${DATANODE0}:10101" SMOKETESTRESULT=$? if (( $SMOKETESTRESULT != 0 )) then - echo "FB-1270 test complete with test failures" + echo "smoke tests complete with test failures" else - echo "FB-1270 test complete" + echo "smoke tests complete" fi exit $SMOKETESTRESULT diff --git a/qa/scripts/utilCluster.sh b/qa/scripts/utilCluster.sh index 95215c93c..e083e082e 100644 --- a/qa/scripts/utilCluster.sh +++ b/qa/scripts/utilCluster.sh @@ -179,6 +179,7 @@ setupIngestNode() { ssh -A -i ~/.ssh/gitlab-featurebase-ci.pem -o StrictHostKeyChecking=no ec2-user@${NODEIP} "pip3 install -U pytest" ssh -A -i ~/.ssh/gitlab-featurebase-ci.pem -o StrictHostKeyChecking=no ec2-user@${NODEIP} "pip3 install -U requests" ssh -A -i ~/.ssh/gitlab-featurebase-ci.pem -o StrictHostKeyChecking=no ec2-user@${NODEIP} "pip3 install -U json" + ssh -A -i ~/.ssh/gitlab-featurebase-ci.pem -o StrictHostKeyChecking=no ec2-user@${NODEIP} "sudo yum install -y jq" } setupDataNodes() { diff --git a/qa/testcases/bug-repros/README b/qa/testcases/bug-repros/README new file mode 100644 index 000000000..730f4ba8b --- /dev/null +++ b/qa/testcases/bug-repros/README @@ -0,0 +1,17 @@ +adding a test case to the bug repros directory??? no problem!!! its a snap!!! + +just make a shell script that does the test you want and name it some thing like: + +fb42069-test.sh + +this will get picked up by run-all.sh and get run automatically!!! + + +# THINGS TO NOTE +- the datanode0 ip will be passed to your script in $1 via qa/scripts/bug_repro_tests.sh + +# ENTHUSIASM +WOW + +shout out to computers for making our lives easier! :) 👍 + diff --git a/qa/testcases/bug-repros/fb-1287-datagen.yaml b/qa/testcases/bug-repros/fb-1287-datagen.yaml new file mode 100644 index 000000000..b4ab21d15 --- /dev/null +++ b/qa/testcases/bug-repros/fb-1287-datagen.yaml @@ -0,0 +1,57 @@ +fields: + - name: "id" + type: uint + step: 1 + distribution: "sequential" + min: 1 + max: 200 + - name: "segid" + type: "int" # (default IntField) + min: 0 + max: 3 + distribution: "zipfian" + s: 1.1 + v: 5.1 + - name: "ts" + type: "timestamp" + min_date: 2006-01-02T15:04:05.001Z # RFC3339Nano + max_date: 2022-01-02T15:04:05.001Z # RFC3339Nano + distribution: "increasing" # only "increasing" is supported right now + min_step_duration: "1ms" + max_step_duration: "200ms" + - name: "lastupdated" + type: "timestamp" + min_date: 2006-01-02T15:04:05.001Z # RFC3339Nano + max_date: 2022-01-02T15:04:05.001Z # RFC3339Nano + distribution: "increasing" # only "increasing" is supported right now + min_step_duration: "1ms" + max_step_duration: "200ms" + - name: "slice" + type: "uint-set" # (default IDArrayField) + min: 0 + max: 35000 + distribution: "zipfian" + s: 1.1 + v: 5.1 + min_num: 1 + max_num: 50 + +idk_params: + primary_key_config: + field: "id" + fields: + segid: + - type: "ID" + ts: + - type: "RecordTime" + layout: "2006-01-02T15:04:05Z" + epoch: 1970-01-01T00:00:00.0Z + name: "na" + - type: "Timestamp" + layout: "2006-01-02T15:04:05Z" + epoch: 1970-01-01T00:00:00.0Z + name: "last_update" + granularity: "s" + slice: + - type: "IDArray" + time_quantum: "D" diff --git a/qa/testcases/bug-repros/fb-1287-test.sh b/qa/testcases/bug-repros/fb-1287-test.sh new file mode 100755 index 000000000..8d275d087 --- /dev/null +++ b/qa/testcases/bug-repros/fb-1287-test.sh @@ -0,0 +1,10 @@ +#!/usr/bin/env bash + +/data/datagen_linux_arm64 -s custom --custom-config=./fb-1287-datagen.yaml --pilosa.index=fb1287 --pilosa.batch-size=100 --pilosa.hosts=$1 + +# if we get an error, exit 1 +if [[ $( curl $1/index/fb1287/query -d 'Rows(segid,from="2022-01-02T15:04",to="2022-04-02T15:04")' | jq '.error' ) != "null" ]]; then + exit 1; +else + exit 0; +fi diff --git a/qa/testcases/FB-1270_repro/test.sh b/qa/testcases/bug-repros/fb1270-test.sh similarity index 100% rename from qa/testcases/FB-1270_repro/test.sh rename to qa/testcases/bug-repros/fb1270-test.sh diff --git a/qa/testcases/bug-repros/run-all.sh b/qa/testcases/bug-repros/run-all.sh new file mode 100755 index 000000000..ac27d85ce --- /dev/null +++ b/qa/testcases/bug-repros/run-all.sh @@ -0,0 +1,8 @@ +#!/usr/bin/env bash + +set -eou pipefail + +for file in `ls *-test.sh`; do + echo "running $file"; + ./$file "$@" +done diff --git a/time.go b/time.go index 945d87106..fb8722fe9 100644 --- a/time.go +++ b/time.go @@ -410,7 +410,7 @@ func minMaxViews(views []string, q TimeQuantum) (min string, max string) { // Sort the list of views. sort.Strings(views) - // Determine the least significant quantum and set that as the + // Determine the least precise quantum and set that as the // number of string characters to compare against. var chars int if q.HasYear() { @@ -499,5 +499,9 @@ func timeOfView(v string, adj bool) (time.Time, error) { // e.g. the view "string_201901" would return "201901". func viewTimePart(v string) string { parts := strings.Split(v, "_") + if _, err := strconv.Atoi(parts[len(parts)-1]); err != nil { + // it's not a number! + return "" + } return parts[len(parts)-1] } diff --git a/time_internal_test.go b/time_internal_test.go index 967b9fc59..9486c5be3 100644 --- a/time_internal_test.go +++ b/time_internal_test.go @@ -398,3 +398,15 @@ func TestParsePartialTime(t *testing.T) { } } + +func TestViewTimePart(t *testing.T) { + for input, want := range map[string]string{ + "standard": "", + "standard_1234567": "1234567", + "standard1234567": "", + } { + if got := viewTimePart(input); got != want { + t.Errorf("expected %v got %v", want, got) + } + } +}