-
Notifications
You must be signed in to change notification settings - Fork 891
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
Add CI check for unecessary template tests #6191
Add CI check for unecessary template tests #6191
Conversation
6b98e4a
to
dfce181
Compare
@akuzm, @antekresic: please review this pull request.
|
d06beb4
to
bb51db3
Compare
Codecov Report
@@ Coverage Diff @@
## main #6191 +/- ##
==========================================
- Coverage 81.46% 81.40% -0.07%
==========================================
Files 246 246
Lines 56954 56908 -46
Branches 12619 12600 -19
==========================================
- Hits 46398 46326 -72
+ Misses 8175 8171 -4
- Partials 2381 2411 +30 see 45 files with indirect coverage changes 📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
bb51db3
to
eb89b73
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like the idea of having this check. However, could you split the commit into (1) removing the telemetry test and (2) introducing this check?
In addition, the files for the telemetry_stats
test seem to be different for the PostgreSQL versions.
$ diff --from-file ./tsl/test/expected/telemetry_stats-*
717c717
< "num_reltuples": 357
---
> "num_reltuples": 368
747c747
< "num_reltuples": 340
---
> "num_reltuples": 329
823c823
< "uncompressed_row_count": 72,
---
> "uncompressed_row_count": 56,
832c832
< "num_reltuples": 285
---
> "num_reltuples": 312
853c853
< "uncompressed_row_count": 44,
---
> "uncompressed_row_count": 60,
862c862
< "num_reltuples": 296
---
> "num_reltuples": 269
eb89b73
to
c79a2f5
Compare
You're correct @jnidzwetzki ... the output are different, but in another PG16 branch that I'm working they will become equal due to the removal of some multi-node tests, so let's merge here just the new CI check. |
When the regression tests output differs between PG versions we create a template test file (*.sql.in) and add several different output files for each supported version. Over time we support new PG version and deprecate older ones, for example nowadays we're working on PG16 support and removed PG12 so we need to check if we still need template files.
c79a2f5
to
26d5c48
Compare
When the regression tests output differs between PG versions we create a template test (aka *.sql.in) and add several different output files for each supported version.
Over time we support new PG version and deprecate older ones, for example nowadays we're working on PG16 support and removed PG12 so we need to check if we still need template files.
Disable-check: force-changelog-file