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

Make sure pgtap errors get caught #46

Merged
merged 15 commits into from
Aug 23, 2021
Merged

Make sure pgtap errors get caught #46

merged 15 commits into from
Aug 23, 2021

Conversation

vincentsarago
Copy link
Member

@vincentsarago vincentsarago commented Aug 19, 2021

ref: #45

@vincentsarago
Copy link
Member Author

vincentsarago commented Aug 19, 2021

I still don't get how the tests pass on GitHub while on my local machine the added tests returns

{"next": null, "prev": null, "type": "FeatureCollection", "context": {"limit": 10, "matched": 0, "returned": 0}, "features": null}

I'm not quite sure the pgtap tests are being run in CI at all 🤷‍♂️

@vincentsarago
Copy link
Member Author

vincentsarago commented Aug 19, 2021

confirmed

the latest commit shows that the result of the pgtap command returns

Checking if any tests are not ok on db ***localhost:5432/migra_to
...
not ok 66 - Test basic search with fields and sort extension and next token
# Failed test 66: "Test basic search with fields and sort extension and next token"
#     Results differ beginning at row 1:
#         have: ("{""next"": ""pgstac-test-item-0020"", ""prev"": ""pgstac-test-item-0007"", ""type"": ""FeatureCollection"", ""context"": {""limit"": 10, ""matched"": 100, ""returned"": 10}, ""features"": [{""id"": ""pgstac-test-item-0007"", ""properties"": {""datetime"": ""2011-08-17T00:00:00Z"", ""eo:cloud_cover"": 59}}, {""id"": ""pgstac-test-item-0008"", ""properties"": {""datetime"": ""2011-08-17T00:00:00Z"", ""eo:cloud_cover"": 64}}, {""id"": ""pgstac-test-item-0009"", ""properties"": {""datetime"": ""2011-08-17T00:00:00Z"", ""eo:cloud_cover"": 61}}, {""id"": ""pgstac-test-item-0014"", ""properties"": {""datetime"": ""2011-08-16T00:00:00Z"", ""eo:cloud_cover"": 17}}, {""id"": ""pgstac-test-item-0015"", ""properties"": {""datetime"": ""2011-08-16T00:00:00Z"", ""eo:cloud_cover"": 54}}, {""id"": ""pgstac-test-item-0016"", ""properties"": {""datetime"": ""2011-08-16T00:00:00Z"", ""eo:cloud_cover"": 13}}, {""id"": ""pgstac-test-item-0017"", ""properties"": {""datetime"": ""2011-08-16T00:00:00Z"", ""eo:cloud_cover"": 59}}, {""id"": ""pgstac-test-item-0018"", ""properties"": {""datetime"": ""2011-08-16T00:00:00Z"", ""eo:cloud_cover"": 29}}, {""id"": ""pgstac-test-item-0019"", ""properties"": {""datetime"": ""2011-08-16T00:00:00Z"", ""eo:cloud_cover"": 52}}, {""id"": ""pgstac-test-item-0020"", ""properties"": {""datetime"": ""2011-08-16T00:00:00Z"", ""eo:cloud_cover"": 39}}]}")
#         want: ("{""next"": ""pgstac-test-item-0020"", ""prev"": ""pgstac-test-item-0011"", ""type"": ""FeatureCollection"", ""context"": {""limit"": 10, ""matched"": 100, ""returned"": 10}, ""features"": [{""id"": ""pgstac-test-item-0011"", ""properties"": {""datetime"": ""2011-08-17T00:00:00Z"", ""eo:cloud_cover"": 41}}, {""id"": ""pgstac-test-item-0012"", ""properties"": {""datetime"": ""2011-08-17T00:00:00Z"", ""eo:cloud_cover"": 4}}, {""id"": ""pgstac-test-item-0013"", ""properties"": {""datetime"": ""2011-08-17T00:00:00Z"", ""eo:cloud_cover"": 2}}, {""id"": ""pgstac-test-item-0014"", ""properties"": {""datetime"": ""2011-08-16T00:00:00Z"", ""eo:cloud_cover"": 17}}, {""id"": ""pgstac-test-item-0015"", ""properties"": {""datetime"": ""2011-08-16T00:00:00Z"", ""eo:cloud_cover"": 54}}, {""id"": ""pgstac-test-item-0016"", ""properties"": {""datetime"": ""2011-08-16T00:00:00Z"", ""eo:cloud_cover"": 13}}, {""id"": ""pgstac-test-item-0017"", ""properties"": {""datetime"": ""2011-08-16T00:00:00Z"", ""eo:cloud_cover"": 59}}, {""id"": ""pgstac-test-item-0018"", ""properties"": {""datetime"": ""2011-08-16T00:00:00Z"", ""eo:cloud_cover"": 29}}, {""id"": ""pgstac-test-item-0019"", ""properties"": {""datetime"": ""2011-08-16T00:00:00Z"", ""eo:cloud_cover"": 52}}, {""id"": ""pgstac-test-item-0020"", ""properties"": {""datetime"": ""2011-08-16T00:00:00Z"", ""eo:cloud_cover"": 39}}]}")
.....
# Looks like you planned 70 tests but ran 75

but the test

if [[ $(echo "$TESTOUTPUT" | grep '^not') == 0 ]]; then
echo "PGTap tests failed."
echo "$TESTOUTPUT" | grep '^not'
exit 1
else
echo "All PGTap Tests Passed!"
fi
does not work

@vincentsarago vincentsarago changed the title Search should return an empty array Make sure pgtag errors get caught Aug 19, 2021
@vincentsarago vincentsarago marked this pull request as ready for review August 19, 2021 08:54
echo "PGTap tests failed."
echo "$TESTOUTPUT" | grep '^not'
echo "$TESTOUTPUT"
Copy link
Member Author

Choose a reason for hiding this comment

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

Let's print all the output from pgtap to help debug

select '{"next": null, "prev": null, "type": "FeatureCollection", "context": {"limit": 10, "matched": 0, "returned": 0}, "features": []}'::jsonb
$$,
'Test collections search return empty feature not null'
);
Copy link
Member Author

Choose a reason for hiding this comment

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

add tests to prove #45

@vincentsarago
Copy link
Member Author

🥳

we now have 3 tests failing

not ok 66 - Test basic search with fields and sort extension and next token
# Failed test 66: "Test basic search with fields and sort extension and next token"
#     Results differ beginning at row 1:
#         have: ("{""next"": ""pgstac-test-item-0020"", ""prev"": ""pgstac-test-item-0007"", ""type"": ""FeatureCollection"", ""context"": {""limit"": 10, ""matched"": 100, ""returned"": 10}, ""features"": [{""id"": ""pgstac-test-item-0007"", ""properties"": {""datetime"": ""2011-08-17T00:00:00Z"", ""eo:cloud_cover"": 59}}, {""id"": ""pgstac-test-item-0008"", ""properties"": {""datetime"": ""2011-08-17T00:00:00Z"", ""eo:cloud_cover"": 64}}, {""id"": ""pgstac-test-item-0009"", ""properties"": {""datetime"": ""2011-08-17T00:00:00Z"", ""eo:cloud_cover"": 61}}, {""id"": ""pgstac-test-item-0014"", ""properties"": {""datetime"": ""2011-08-16T00:00:00Z"", ""eo:cloud_cover"": 17}}, {""id"": ""pgstac-test-item-0015"", ""properties"": {""datetime"": ""2011-08-16T00:00:00Z"", ""eo:cloud_cover"": 54}}, {""id"": ""pgstac-test-item-0016"", ""properties"": {""datetime"": ""2011-08-16T00:00:00Z"", ""eo:cloud_cover"": 13}}, {""id"": ""pgstac-test-item-0017"", ""properties"": {""datetime"": ""2011-08-16T00:00:00Z"", ""eo:cloud_cover"": 59}}, {""id"": ""pgstac-test-item-0018"", ""properties"": {""datetime"": ""2011-08-16T00:00:00Z"", ""eo:cloud_cover"": 29}}, {""id"": ""pgstac-test-item-0019"", ""properties"": {""datetime"": ""2011-08-16T00:00:00Z"", ""eo:cloud_cover"": 52}}, {""id"": ""pgstac-test-item-0020"", ""properties"": {""datetime"": ""2011-08-16T00:00:00Z"", ""eo:cloud_cover"": 39}}]}")
#         want: ("{""next"": ""pgstac-test-item-0020"", ""prev"": ""pgstac-test-item-0011"", ""type"": ""FeatureCollection"", ""context"": {""limit"": 10, ""matched"": 100, ""returned"": 10}, ""features"": [{""id"": ""pgstac-test-item-0011"", ""properties"": {""datetime"": ""2011-08-17T00:00:00Z"", ""eo:cloud_cover"": 41}}, {""id"": ""pgstac-test-item-0012"", ""properties"": {""datetime"": ""2011-08-17T00:00:00Z"", ""eo:cloud_cover"": 4}}, {""id"": ""pgstac-test-item-0013"", ""properties"": {""datetime"": ""2011-08-17T00:00:00Z"", ""eo:cloud_cover"": 2}}, {""id"": ""pgstac-test-item-0014"", ""properties"": {""datetime"": ""2011-08-16T00:00:00Z"", ""eo:cloud_cover"": 17}}, {""id"": ""pgstac-test-item-0015"", ""properties"": {""datetime"": ""2011-08-16T00:00:00Z"", ""eo:cloud_cover"": 54}}, {""id"": ""pgstac-test-item-0016"", ""properties"": {""datetime"": ""2011-08-16T00:00:00Z"", ""eo:cloud_cover"": 13}}, {""id"": ""pgstac-test-item-0017"", ""properties"": {""datetime"": ""2011-08-16T00:00:00Z"", ""eo:cloud_cover"": 59}}, {""id"": ""pgstac-test-item-0018"", ""properties"": {""datetime"": ""2011-08-16T00:00:00Z"", ""eo:cloud_cover"": 29}}, {""id"": ""pgstac-test-item-0019"", ""properties"": {""datetime"": ""2011-08-16T00:00:00Z"", ""eo:cloud_cover"": 52}}, {""id"": ""pgstac-test-item-0020"", ""properties"": {""datetime"": ""2011-08-16T00:00:00Z"", ""eo:cloud_cover"": 39}}]}")

not ok 67 - Test basic search with fields and sort extension and prev token
# Failed test 67: "Test basic search with fields and sort extension and prev token"
#     Results differ beginning at row 1:
#         have: ("{""next"": ""pgstac-test-item-0013"", ""prev"": ""pgstac-test-item-0001"", ""type"": ""FeatureCollection"", ""context"": {""limit"": 10, ""matched"": 100, ""returned"": 8}, ""features"": [{""id"": ""pgstac-test-item-0001"", ""properties"": {""datetime"": ""2011-08-25T00:00:00Z"", ""eo:cloud_cover"": 89}}, {""id"": ""pgstac-test-item-0002"", ""properties"": {""datetime"": ""2011-08-25T00:00:00Z"", ""eo:cloud_cover"": 33}}, {""id"": ""pgstac-test-item-0003"", ""properties"": {""datetime"": ""2011-08-25T00:00:00Z"", ""eo:cloud_cover"": 28}}, {""id"": ""pgstac-test-item-0004"", ""properties"": {""datetime"": ""2011-08-24T00:00:00Z"", ""eo:cloud_cover"": 23}}, {""id"": ""pgstac-test-item-0005"", ""properties"": {""datetime"": ""2011-08-24T00:00:00Z"", ""eo:cloud_cover"": 3}}, {""id"": ""pgstac-test-item-0006"", ""properties"": {""datetime"": ""2011-08-24T00:00:00Z"", ""eo:cloud_cover"": 100}}, {""id"": ""pgstac-test-item-0012"", ""properties"": {""datetime"": ""2011-08-17T00:00:00Z"", ""eo:cloud_cover"": 4}}, {""id"": ""pgstac-test-item-0013"", ""properties"": {""datetime"": ""2011-08-17T00:00:00Z"", ""eo:cloud_cover"": 2}}]}")
#         want: ("{""next"": ""pgstac-test-item-0010"", ""prev"": null, ""type"": ""FeatureCollection"", ""context"": {""limit"": 10, ""matched"": 100, ""returned"": 10}, ""features"": [{""id"": ""pgstac-test-item-0001"", ""properties"": {""datetime"": ""2011-08-25T00:00:00Z"", ""eo:cloud_cover"": 89}}, {""id"": ""pgstac-test-item-0002"", ""properties"": {""datetime"": ""2011-08-25T00:00:00Z"", ""eo:cloud_cover"": 33}}, {""id"": ""pgstac-test-item-0003"", ""properties"": {""datetime"": ""2011-08-25T00:00:00Z"", ""eo:cloud_cover"": 28}}, {""id"": ""pgstac-test-item-0004"", ""properties"": {""datetime"": ""2011-08-24T00:00:00Z"", ""eo:cloud_cover"": 23}}, {""id"": ""pgstac-test-item-0005"", ""properties"": {""datetime"": ""2011-08-24T00:00:00Z"", ""eo:cloud_cover"": 3}}, {""id"": ""pgstac-test-item-0006"", ""properties"": {""datetime"": ""2011-08-24T00:00:00Z"", ""eo:cloud_cover"": 100}}, {""id"": ""pgstac-test-item-0007"", ""properties"": {""datetime"": ""2011-08-17T00:00:00Z"", ""eo:cloud_cover"": 59}}, {""id"": ""pgstac-test-item-0008"", ""properties"": {""datetime"": ""2011-08-17T00:00:00Z"", ""eo:cloud_cover"": 64}}, {""id"": ""pgstac-test-item-0009"", ""properties"": {""datetime"": ""2011-08-17T00:00:00Z"", ""eo:cloud_cover"": 61}}, {""id"": ""pgstac-test-item-0010"", ""properties"": {""datetime"": ""2011-08-17T00:00:00Z"", ""eo:cloud_cover"": 31}}]}")

not ok 75 - Test collections search return empty feature not null
# Failed test 75: "Test collections search return empty feature not null"
#     Results differ beginning at row 1:
#         have: ("{""next"": null, ""prev"": null, ""type"": ""FeatureCollection"", ""context"": {""limit"": 10, ""matched"": 0, ""returned"": 0}, ""features"": null}")
#         want: ("{""next"": null, ""prev"": null, ""type"": ""FeatureCollection"", ""context"": {""limit"": 10, ""matched"": 0, ""returned"": 0}, ""features"": []}")

test 75, shows the behaviour of #45. The two others will need deeper looks

@vincentsarago vincentsarago changed the title Make sure pgtag errors get caught Make sure pgtap errors get caught Aug 19, 2021
@@ -845,7 +843,7 @@ context := jsonb_strip_nulls(jsonb_build_object(

collection := jsonb_build_object(
'type', 'FeatureCollection',
'features', out_records,
'features', coalesce(out_records, '[]'::jsonb),
Copy link
Member Author

Choose a reason for hiding this comment

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

simple fix to make sure features is at least set to [].

I couldn't track down where in https://github.com/stac-utils/pgstac/pull/46/files#diff-3c3e93de12c078e7f46e264188fcc2ba82eefd77be73795f964ad132d89af285R835 the transformation from [] to null was 🤷

Copy link
Member Author

Choose a reason for hiding this comment

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

😬 tests still fails 🤷‍♂️
😭 but it works locally 😭

@bitner
Copy link
Collaborator

bitner commented Aug 23, 2021

@vincentsarago I think one of your tests was failing due to not having regenerated the migrations files (/scripts/stageversion). The other two were failing because there was an additional place where the error I fixed before I left with "sort" vs "sortby" was still wrong. When looking for this, I also fixed and added tests for a bug where I had used "id" rather than "ids".

@bitner bitner merged commit 1c0694e into main Aug 23, 2021
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.

None yet

2 participants