Skip to content

Conversation

@hermanschaaf
Copy link
Member

  • Skipping of null values in lists when doing comparisons is necessary for BigQuery tests to pass right now, because BigQuery does not support null values in repeated columns. To keep the migration to Arrow backwards-compatible, we are opting to strip nulls in that case (for now), but we still want our automated tests to pass.
  • This also refactors the test options. Previously you had to call destination.PluginTestSuiteRunner with options from the schema package, which is a bit unusual and counter-intuitive, and makes it hard to extend with options that relate specifically to the test suite runner. This changes it so that the options passed in are from the destination package and passed through to the schema package. It's a breaking change, but this functionality hasn't been out for long, will be easy to update, and only affects tests, so I'm hoping users will forgive us for this one.

@github-actions
Copy link

github-actions bot commented May 19, 2023

⏱️ Benchmark results

Comparing with 6967929

  • DefaultConcurrencyDFS-2 resources/s: 10,512 ⬇️ 3.03% decrease vs. 6967929
  • DefaultConcurrencyRoundRobin-2 resources/s: 12,428 ⬆️ 9.99% increase vs. 6967929
  • Glob-2 ns/op: 204.1 ⬆️ 3.14% increase vs. 6967929
  • TablesWithChildrenDFS-2 resources/s: 28,210 ⬆️ 6.62% increase vs. 6967929
  • TablesWithChildrenRoundRobin-2 resources/s: 25,811 ⬇️ 9.98% decrease vs. 6967929
  • TablesWithRateLimitingDFS-2 resources/s: 28.47 ⬆️ 0.56% increase vs. 6967929
  • TablesWithRateLimitingRoundRobin-2 resources/s: 888.5 ⬆️ 5.63% increase vs. 6967929

@codecov
Copy link

codecov bot commented May 19, 2023

Codecov Report

Patch coverage: 58.82% and project coverage change: -0.14 ⚠️

Comparison is base (6967929) 47.33% compared to head (1286b5a) 47.19%.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #892      +/-   ##
==========================================
- Coverage   47.33%   47.19%   -0.14%     
==========================================
  Files          55       55              
  Lines        5049     5013      -36     
==========================================
- Hits         2390     2366      -24     
+ Misses       2407     2397      -10     
+ Partials      252      250       -2     
Impacted Files Coverage Δ
schema/testdata.go 44.83% <58.82%> (-2.10%) ⬇️

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

Copy link
Contributor

@yevgenypats yevgenypats left a comment

Choose a reason for hiding this comment

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

Nice. Need that also for mongo but for different option - changing the timestamp precision (mongodb only supports milliseconds).

@kodiakhq kodiakhq bot merged commit bc3c251 into main May 20, 2023
@kodiakhq kodiakhq bot deleted the refactor-tests branch May 20, 2023 06:08
kodiakhq bot pushed a commit that referenced this pull request May 21, 2023
🤖 I have created a release *beep* *boop*
---


## [3.6.0](v3.5.2...v3.6.0) (2023-05-21)


### Features

* Add precision options for dest testing ([#893](#893)) ([faacca6](faacca6))
* Refactor test options and allow skipping of nulls in lists ([#892](#892)) ([bc3c251](bc3c251))


### Bug Fixes

* Add null-row case for append-only tests ([#889](#889)) ([6967929](6967929))
* Tighter Arrow test cases ([#891](#891)) ([c7f2546](c7f2546))

---
This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants