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

Implement tests as GitHub workflow #23

Merged
merged 28 commits into from
Dec 4, 2024
Merged

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 pnwpedro, jrodewig and a team 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
.gitignore Outdated Show resolved Hide resolved
Comment on lines +41 to +44
### Fauna ###
.fauna*

Copy link
Contributor

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.

Copy link
Contributor

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.

test/validate.sh Outdated
Copy link
Contributor

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. 🤔

Copy link
Contributor

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.

2 participants