Skip to content

[cleanup] moving test sql to a yaml file for better readability#9580

Merged
walterddr merged 2 commits into
apache:masterfrom
walterddr:ssb_yaml
Oct 12, 2022
Merged

[cleanup] moving test sql to a yaml file for better readability#9580
walterddr merged 2 commits into
apache:masterfrom
walterddr:ssb_yaml

Conversation

@walterddr
Copy link
Copy Markdown
Contributor

  • hard-coded string needs to follow checkstyle. also it cannot be directly copied.
  • didn't use json b/c it doesn't support multiline as well as yaml

@walterddr walterddr marked this pull request as ready for review October 12, 2022 21:05
Copy link
Copy Markdown
Contributor

@agavra agavra left a comment

Choose a reason for hiding this comment

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

😍 yessss! can we also do that for QueryRunnerTest and make this a generic thing we can use in lots of tests 🙏

@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented Oct 12, 2022

Codecov Report

Merging #9580 (3de055d) into master (00d35a2) will increase coverage by 0.04%.
The diff coverage is n/a.

@@             Coverage Diff              @@
##             master    #9580      +/-   ##
============================================
+ Coverage     68.75%   68.79%   +0.04%     
- Complexity     4912     5253     +341     
============================================
  Files          1934     1934              
  Lines        103290   103290              
  Branches      15695    15695              
============================================
+ Hits          71016    71061      +45     
+ Misses        27187    27156      -31     
+ Partials       5087     5073      -14     
Flag Coverage Δ
integration1 26.02% <ø> (+0.05%) ⬆️
unittests1 67.38% <ø> (-0.02%) ⬇️
unittests2 15.62% <ø> (+0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
.../impl/dictionary/BaseOffHeapMutableDictionary.java 81.33% <0.00%> (-3.34%) ⬇️
...gregation/function/StUnionAggregationFunction.java 73.52% <0.00%> (-2.95%) ⬇️
...core/operator/docvalsets/TransformBlockValSet.java 59.43% <0.00%> (-2.84%) ⬇️
...inot/core/operator/filter/FilterOperatorUtils.java 87.50% <0.00%> (-2.50%) ⬇️
...perator/filter/SortedIndexBasedFilterOperator.java 83.80% <0.00%> (-1.91%) ⬇️
...controller/helix/core/minion/PinotTaskManager.java 66.56% <0.00%> (-1.47%) ⬇️
...ery/optimizer/filter/MergeEqInFilterOptimizer.java 92.59% <0.00%> (-1.24%) ⬇️
...org/apache/pinot/core/common/ObjectSerDeUtils.java 90.71% <0.00%> (-0.80%) ⬇️
...core/query/reduce/ExplainPlanDataTableReducer.java 82.99% <0.00%> (-0.69%) ⬇️
...ery/optimizer/filter/NumericalFilterOptimizer.java 80.89% <0.00%> (-0.57%) ⬇️
... and 22 more

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

Comment thread pinot-integration-tests/src/test/resources/ssb/ssb_query_set.yaml Outdated
@walterddr walterddr merged commit c581fd9 into apache:master Oct 12, 2022
@walterddr
Copy link
Copy Markdown
Contributor Author

😍 yessss! can we also do that for QueryRunnerTest and make this a generic thing we can use in lots of tests 🙏

good idea. although there's more to improve in QueryRunnerTest for example

  • better data randomness, currently they are hard-coded
  • better data type coverage, current only 2 numeric, 2 string and 1 timestamp
  • better table schema, currently all are the same.
    let's create an issue to track this?

@walterddr walterddr deleted the ssb_yaml branch December 6, 2023 16:25
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.

4 participants