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.

Sign up for GitHub

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

Jump to bottom

test(warehouse): pump more test for warehouse module #2460

Merged
merged 31 commits into from Oct 6, 2022
Merged

test(warehouse): pump more test for warehouse module #2460

merged 31 commits into from Oct 6, 2022

Conversation

achettyiitr
Copy link
Member

@achettyiitr achettyiitr commented Sep 20, 2022 •
edited

Description

Some code cleanup.

Added a new set of test cases for the warehouse module around:

  • Warehouse schemas
  • Warehouse GRPC modules
  • Warehouse archiving
  • Warehouse table uploads
  • Warehouse uploads test

Notion Ticket

https://www.notion.so/rudderstacks/Warehouse-tests-97aa9f5fba024556b1fe2e6f3d604562

Security

  • The code changed/added as part of this pull request won't create any security issues with how the software is being used.

@achettyiitr achettyiitr self-assigned this Sep 20, 2022
@codecov
Copy link

codecov bot commented Sep 20, 2022 •
edited

Codecov Report

Base: 41.60% // Head: 45.10% // Increases project coverage by +3.50% 🎉

Coverage data is based on head (773f5ba) compared to base (cf661fd).
Patch coverage: 63.04% of modified lines in pull request are covered.

Additional details and impacted files 
@@            Coverage Diff             @@
##           master    #2460      +/-   ##
==========================================
+ Coverage   41.60%   45.10%   +3.50%     
==========================================
  Files         175      175              
  Lines       36298    36298              
==========================================
+ Hits        15101    16372    +1271     
+ Misses      20255    18872    -1383     
- Partials      942     1054     +112
Impacted Files Coverage Δ
warehouse/schema.go 55.55% <12.50%> (+45.79%) ⬆️
warehouse/warehouse.go 9.03% <83.33%> (+5.92%) ⬆️
warehouse/warehousegrpc.go 96.15% <87.50%> (+96.15%) ⬆️
warehouse/api.go 70.42% <88.88%> (+70.42%) ⬆️
warehouse/archiver.go 49.54% <100.00%> (+46.39%) ⬆️
warehouse/retry.go 87.58% <100.00%> (+87.58%) ⬆️
warehouse/stats.go 62.50% <100.00%> (+62.50%) ⬆️
warehouse/upload.go 17.23% <0.00%> (+10.74%) ⬆️
... and 8 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

@achettyiitr achettyiitr changed the title chore(warehouse): add test for warehouse grpc chore(warehouse): pump more test for warehouse modules Sep 21, 2022
@achettyiitr achettyiitr changed the title chore(warehouse): pump more test for warehouse modules chore(warehouse): pump more test for warehouse module Sep 21, 2022
@achettyiitr achettyiitr changed the title chore(warehouse): pump more test for warehouse module test(warehouse): pump more test for warehouse module Sep 22, 2022
@achettyiitr
review comments for warehouseGRPC
6689e0b
@achettyiitr
Review comments:
632722a 
1. use warehouseGRPC.
2. use testdata for storing sql's.
3. use proper mocking for stats using defer after the test spec is completed.
@achettyiitr
Merge branch 'master' of github.com:rudderlabs/rudder-server into cho…
57f2d35 
…re.testForWarehouseGrpc
lvrach
lvrach approved these changes Oct 4, 2022
View reviewed changes
Copy link
Member

@lvrach lvrach left a comment

Choose a reason for hiding this comment

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

Approve, with some notes to revisit later:

  • Directly populating the database using SQL statements as a precondition for a test
  • Usage of config or DI in place of .Setenv, as environment variables can cause harm when a test is run concurrently.

@achettyiitr achettyiitr merged commit 58671c1 into master Oct 6, 2022
@rudder-server-bot rudder-server-bot mentioned this pull request Oct 6, 2022
Merged
@github-actions github-actions bot deleted the chore.testForWarehouseGrpc branch December 5, 2022 02:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lvrach lvrach lvrach approved these changes

@Jayachand Jayachand Jayachand approved these changes

@abhimanyubabbar abhimanyubabbar Awaiting requested review from abhimanyubabbar

Assignees

@achettyiitr achettyiitr

Labels
Ready for Review warehouse-team with tests
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@achettyiitr @lvrach @Jayachand