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

add schema tests to the shell pipeline #318

Merged
merged 2 commits into from
Oct 19, 2023
Merged

add schema tests to the shell pipeline #318

merged 2 commits into from
Oct 19, 2023

Conversation

fauna-chase
Copy link
Contributor

I validated that the task works as expected with fly execute. Once I verify it is working as expected in the pipeline I'll add it as a passed constraint to the publish job. I'll also move the integ tests to the test job, that way we will always auto run the tests and publish can be executed with the latest version that passed the tests (or any selected version that passed the tests)

I validated that the task works as expected with fly execute.  Once I
verify it is working as expected in the pipeline I'll add it as a passed
constraint to the publish job.  I'll also move the integ tests to the
test job, that way we will always auto run the tests and publish can be
executed with the latest version that passed the tests (or any selected
version that passed the tests)
@fauna-chase fauna-chase requested a review from macmv October 18, 2023 21:31
Copy link
Contributor

@macmv macmv left a comment

Choose a reason for hiding this comment

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

looks good, except for the || comment.

might be nice to put this in the github PR ci too.

Comment on lines 30 to 36
if (collNames.length != expectedCollNames.length && !collNames.every((elem, idx) => elem === expectedCollNames[idx])) {
throw new Error(`Schema collections do not match actual: ${collNames} expected: ${expectedCollNames}`)
}
const funcNames = await execPaginated("Function.all().map(.name)")
if (funcNames.length != expectedFuncNames.length && !funcNames.every((elem, idx) => elem === expectedFuncNames[idx])) {
throw new Error(`Schema functions do not match actual: ${collNames} expected: ${expectedCollNames}`)
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
if (collNames.length != expectedCollNames.length && !collNames.every((elem, idx) => elem === expectedCollNames[idx])) {
throw new Error(`Schema collections do not match actual: ${collNames} expected: ${expectedCollNames}`)
}
const funcNames = await execPaginated("Function.all().map(.name)")
if (funcNames.length != expectedFuncNames.length && !funcNames.every((elem, idx) => elem === expectedFuncNames[idx])) {
throw new Error(`Schema functions do not match actual: ${collNames} expected: ${expectedCollNames}`)
}
if (collNames.length != expectedCollNames.length || !collNames.every((elem, idx) => elem === expectedCollNames[idx])) {
throw new Error(`Schema collections do not match actual: ${collNames} expected: ${expectedCollNames}`)
}
const funcNames = await execPaginated("Function.all().map(.name)")
if (funcNames.length != expectedFuncNames.length || !funcNames.every((elem, idx) => elem === expectedFuncNames[idx])) {
throw new Error(`Schema functions do not match actual: ${collNames} expected: ${expectedCollNames}`)
}

@fauna-chase
Copy link
Contributor Author

looks good, except for the || comment.

might be nice to put this in the github PR ci too.

thats a good callout, I think I'll need to do a bit of tweaking there to use the docker image and create a fresh db and such so we don't run into any issues with them running concurrently. Definitely seems useful to get them on the prs in that manner.

@fauna-chase fauna-chase merged commit aa528a6 into main Oct 19, 2023
@fauna-chase fauna-chase deleted the schema-tests branch October 19, 2023 01:20
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