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

e2e groupby conditional scenarios #1807

Merged
merged 1 commit into from
Sep 15, 2023
Merged

Conversation

itsmekumari
Copy link
Contributor

@itsmekumari itsmekumari commented Aug 29, 2023

Design document available in PR review folder.

@Aryan-Verma Aryan-Verma added the build Trigger unit test build label Sep 1, 2023
@sau42shri sau42shri requested a review from vanathi-g September 1, 2023 12:41
Copy link
Contributor

@vanathi-g vanathi-g left a comment

Choose a reason for hiding this comment

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

Also, just a general question, why were minIf and anyIf kept in a separate scenario? Why not keep them in the first scenario itself?

Then Validate output file generated by file sink plugin "fileSinkTargetBucket" is equal to expected output file "groupByTest5OutputFile"

@GROUP_BY_TEST @FILE_SINK_TEST
Scenario: To verify complete flow of data extract and transfer from File source to File sink with GroupBy plugin using set of aggregates
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: update scenario description to mention which aggregates are being tested (count, sum, max)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated scenario description.

@itsmekumari
Copy link
Contributor Author

Also, just a general question, why were minIf and anyIf kept in a separate scenario? Why not keep them in the first scenario itself?

Added minif and anyif aggregators in first scenario.

@itsmekumari itsmekumari force-pushed the e2e_groupby branch 5 times, most recently from f3e218e to 3502087 Compare September 12, 2023 16:27
Copy link
Contributor

@vanathi-g vanathi-g left a comment

Choose a reason for hiding this comment

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

Seems like the build failure might be flaky behaviour, please validate that once. LGTM otherwise. Ensure e2e test build works before merging

@itsmekumari itsmekumari force-pushed the e2e_groupby branch 5 times, most recently from 8fbb96b to 32db009 Compare September 14, 2023 10:05
@vanathi-g
Copy link
Contributor

vanathi-g commented Sep 15, 2023

e2e test failure seems to be due to "Verify Distinct plugin validation errors for incorrect data in Fields" (src/e2e-test/features/distinctplugin/DistinctErrorScenarios.feature:5) scenario. Merging since it is unrelated.

@vanathi-g vanathi-g merged commit bc1929b into cdapio:develop Sep 15, 2023
3 of 4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
build Trigger unit test build
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants