Skip to content

Conversation

@findgriffin
Copy link
Contributor

@findgriffin findgriffin commented Nov 15, 2024

Add scripts to setup DB using local container and validate sample app:

All of this work should be copyable to the other sample apps (e.g. Python).

  1. Setup GitHub Actions workflow in .github to install JVM, fauna docker container, and fauna-shell.
  2. Use fauna shell in test/setup.sh to configure the test database.
  3. Run curl requests against the sample app in test/validate.sh

@findgriffin findgriffin mentioned this pull request Nov 21, 2024
@findgriffin findgriffin requested review from a team, jrodewig and pnwpedro and removed request for jrodewig November 21, 2024 23:15
@findgriffin findgriffin changed the title Install fauna-shell. Implement tests as GitHub workflow Nov 22, 2024
@pnwpedro pnwpedro requested a review from cynicaljoy November 22, 2024 12:09
Comment on lines +41 to +44

### Fauna ###
.fauna*

Choose a reason for hiding this comment

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

We should check with the Fauna shell team to be sure no files are intended to be checked in when working on a team.

Choose a reason for hiding this comment

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

from the discussion yesterday it sounds like we want to produce a test suite that's more native to the sample app. So JUnit in this case.

Choose a reason for hiding this comment

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

if we're producing JUnit this doesn't matter much, but providing feedback for general bash improvements.

  1. I don't think we should write files unless absolutely required
  2. use a var for all of the common curl options, and don't use shorthand in scripts (we still have a lot of this everywhere, but trying to be better about it)
  3. separate flags to new lines

result:

CURL_OPTS='--silent --header "Accept: application/json" --header "Content-Type: application/json"'

PAGE_ONE_=$(curl $CURL_OPTS \
    --retry-all-errors \
    --connect-timeout 5 \
    --max-time 10 \
    --retry 5 \
    --retry-delay 10 \
    --retry-max-time 60 \
    "$ENDPOINT/products?pageSize=1")

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I agree with all 3 points here.

On points 1 and 2, I tried to use the syntax you proposed here, but it doesn't seem to work. CURL_OPTS is not getting split into multiple arguments, and it seems to hang when I try and pass the output of curl into a variable. 🤔

Choose a reason for hiding this comment

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

you could eval, but I actually think this pattern would be best:

CURL_OPTS=(
  "--silent"
  "--show-error"
  "--fail"
  "--connect-timeout" "2"
  "--max-time" "2"
)

# just throwing in a verbose to demonstrate we can append more opts if needed
curl "${CURL_OPTS[@]}" --verbose "$ENDPOINT/products?pageSize=1"

@findgriffin findgriffin merged commit 7ebf3ee into main Dec 4, 2024
1 check passed
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.

3 participants