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

Disable tests by default if tools are not found #3468

Merged
merged 1 commit into from
Aug 11, 2021

Conversation

mkindahl
Copy link
Contributor

@mkindahl mkindahl commented Aug 10, 2021

If tools like pg_regress or pg_isolation_regress are not found, an
error is generated telling you that you need to disable the tests
explicitly using REGRESS_CHECKS. This is an inconvenience for the
user since in most cases you just want to build the system with what
you have.

This commit changes that by automatically disabling regression tests,
isolation tests, TAP tests, and formatting targets when tools cannot be
found, which means that users do not normally have to disable tests
explicitly. A notice is still printed that the targets are not added.

In addition, the commit adds an option REQUIRE_ALL_TESTS for the rare
case when you want to make sure that all tests are indeed executed (for
example, in CI runs), which by default is off.

@mkindahl mkindahl self-assigned this Aug 10, 2021
@mkindahl mkindahl changed the title Add option to disable clang-format Add option to disable code formatting Aug 10, 2021
@codecov
Copy link

codecov bot commented Aug 10, 2021

Codecov Report

Merging #3468 (b14ebbc) into master (e1c5a98) will decrease coverage by 55.81%.
The diff coverage is n/a.

❗ Current head b14ebbc differs from pull request most recent head d5a09de. Consider uploading reports for the commit d5a09de to get more accurate results
Impacted file tree graph

@@             Coverage Diff             @@
##           master    #3468       +/-   ##
===========================================
- Coverage   90.73%   34.92%   -55.82%     
===========================================
  Files         212      212               
  Lines       36258    36258               
===========================================
- Hits        32900    12664    -20236     
- Misses       3358    23594    +20236     
Impacted Files Coverage Δ
src/compat.h 0.00% <0.00%> (-100.00%) ⬇️
tsl/src/telemetry.c 0.00% <0.00%> (-100.00%) ⬇️
tsl/src/remote/utils.c 0.00% <0.00%> (-100.00%) ⬇️
tsl/src/compression/utils.h 0.00% <0.00%> (-100.00%) ⬇️
tsl/test/src/remote/async.c 0.00% <0.00%> (-100.00%) ⬇️
tsl/src/nodes/gapfill/locf.c 0.00% <0.00%> (-100.00%) ⬇️
tsl/src/remote/txn_resolve.c 0.00% <0.00%> (-100.00%) ⬇️
tsl/test/src/remote/txn_id.c 0.00% <0.00%> (-100.00%) ⬇️
tsl/src/remote/data_fetcher.h 0.00% <0.00%> (-100.00%) ⬇️
tsl/src/nodes/gapfill/gapfill.c 0.00% <0.00%> (-100.00%) ⬇️
... and 166 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e1c5a98...d5a09de. Read the comment docs.

@mkindahl mkindahl marked this pull request as ready for review August 10, 2021 08:12
@mkindahl mkindahl requested a review from a team as a code owner August 10, 2021 08:12
@mkindahl mkindahl requested review from pmwkaa, afiskon and nikkhils and removed request for a team August 10, 2021 08:12
Copy link
Member

@svenklemm svenklemm left a comment

Choose a reason for hiding this comment

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

make format is entirely optional for the build process. not sure why we would need an option to enable it since it's sufficient and much more user friendly to check at execution time for the availabilty. this is also much more friendly to packagers.

@mkindahl
Copy link
Contributor Author

make format is entirely optional for the build process. not sure why we would need an option to enable it since it's sufficient and much more user friendly to check at execution time for the availabilty. this is also much more friendly to packagers.

If clang-format does not exist, a build rule to run clang-format using Docker is created instead. We could check for availability of Docker as well and just ignore the situation if docker is not available either, which would indeed be more user-friendly.

However, that raises the issue why we do not do the same when pg_regress is not found. In this case, we generate an error and it needs to be explicitly disabled. If user-friendliness is a goal, then we should disable regress tests when pg_regress is not found instead of erroring out. (And the same reasoning applies to pg_isolation_regress, and TAP tests.)

@mkindahl mkindahl force-pushed the clang_format_flag branch 2 times, most recently from b14ebbc to 817afcf Compare August 11, 2021 08:13
@mkindahl mkindahl changed the title Add option to disable code formatting Disable tests by default if tools are not found Aug 11, 2021
@mkindahl
Copy link
Contributor Author

The pull request is updated to by default disable tests if they are not found. It also moves some of the checks for programs to the topmost CMakeLists.txt.

appveyor.yml Outdated Show resolved Hide resolved
@mkindahl mkindahl marked this pull request as draft August 11, 2021 09:29
If tools like `pg_regress` or `pg_isolation_regress` are not found, an
error is generated telling you that you need to disable the tests
explicitly using `REGRESS_CHECKS`. This is an inconvenience for the
user since in most cases you just want to build the system with what
you have.

This commit changes that by automatically disabling regression tests,
isolation tests, TAP tests, and formatting targets when tools cannot be
found, which means that users do not normally have to disable tests
explicitly. A notice is still printed that the targets are not added.

In addition, the commit adds an option `REQUIRE_ALL_TESTS` for the rare
case when you want to make sure that all tests are indeed executed (for
example, in CI runs), which by default is off.
@mkindahl mkindahl marked this pull request as ready for review August 11, 2021 11:25
@mkindahl mkindahl merged commit bff87a8 into timescale:master Aug 11, 2021
@mkindahl mkindahl deleted the clang_format_flag branch August 11, 2021 13:28
mkindahl added a commit to mkindahl/timescaledb that referenced this pull request Aug 19, 2021
This release contains bug fixes since the 2.4.0 release. We deem it
high priority to upgrade since it is needed to support PostgreSQL 12.8
and 13.4.

**Bugfixes**
* timescale#3430 Fix havingqual processing for continuous aggregates
* timescale#3468 Disable tests by default if tools are not found
* timescale#3468 Fix crash while tracking alter table commands
* timescale#3494 Improve error message when adding data nodes
* timescale#3498 Fix continuous agg bgw job failure for PG 12.8 and 13.4

**Thanks**
* @brianbenns for reporting a segfault with continuous aggregates
mkindahl added a commit to mkindahl/timescaledb that referenced this pull request Aug 19, 2021
This release contains bug fixes since the 2.4.0 release. We deem it
high priority to upgrade since it is needed to support PostgreSQL 12.8
and 13.4.

**Bugfixes**
* timescale#3430 Fix havingqual processing for continuous aggregates
* timescale#3468 Disable tests by default if tools are not found
* timescale#3468 Fix crash while tracking alter table commands
* timescale#3494 Improve error message when adding data nodes
* timescale#3498 Fix continuous agg bgw job failure for PG 12.8 and 13.4

**Thanks**
* @brianbenns for reporting a segfault with continuous aggregates
mkindahl added a commit to mkindahl/timescaledb that referenced this pull request Aug 19, 2021
This release contains bug fixes since the 2.4.0 release. We deem it
high priority to upgrade since it is needed to support PostgreSQL 12.8
and 13.4.

**Bugfixes**
* timescale#3430 Fix havingqual processing for continuous aggregates
* timescale#3468 Disable tests by default if tools are not found
* timescale#3468 Fix crash while tracking alter table commands
* timescale#3494 Improve error message when adding data nodes
* timescale#3498 Fix continuous agg bgw job failure for PG 12.8 and 13.4

**Thanks**
* @brianbenns for reporting a segfault with continuous aggregates
mkindahl added a commit to mkindahl/timescaledb that referenced this pull request Aug 19, 2021
This release contains bug fixes since the 2.4.0 release. We deem it
high priority to upgrade since it is needed to support PostgreSQL 12.8
and 13.4.

**Bugfixes**
* timescale#3430 Fix havingqual processing for continuous aggregates
* timescale#3468 Disable tests by default if tools are not found
* timescale#3468 Fix crash while tracking alter table commands
* timescale#3494 Improve error message when adding data nodes
* timescale#3498 Fix continuous agg bgw job failure for PG 12.8 and 13.4

**Thanks**
* @brianbenns for reporting a segfault with continuous aggregates
mkindahl added a commit to mkindahl/timescaledb that referenced this pull request Aug 19, 2021
This release contains bug fixes since the 2.4.0 release. We deem it
high priority to upgrade since it is needed to support PostgreSQL 12.8
and 13.4.

**Bugfixes**
* timescale#3430 Fix havingqual processing for continuous aggregates
* timescale#3468 Disable tests by default if tools are not found
* timescale#3468 Fix crash while tracking alter table commands
* timescale#3494 Improve error message when adding data nodes
* timescale#3498 Fix continuous agg bgw job failure for PG 12.8 and 13.4

**Thanks**
* @brianbenns for reporting a segfault with continuous aggregates
mkindahl added a commit to mkindahl/timescaledb that referenced this pull request Aug 19, 2021
This release contains bug fixes since the 2.4.0 release. We deem it
high priority to upgrade since it is needed to support PostgreSQL 12.8
and 13.4.

**Bugfixes**
* timescale#3430 Fix havingqual processing for continuous aggregates
* timescale#3468 Disable tests by default if tools are not found
* timescale#3468 Fix crash while tracking alter table commands
* timescale#3494 Improve error message when adding data nodes
* timescale#3498 Fix continuous agg bgw job failure for PG 12.8 and 13.4

**Thanks**
* @brianbenns for reporting a segfault with continuous aggregates
mkindahl added a commit to mkindahl/timescaledb that referenced this pull request Aug 19, 2021
This release contains bug fixes since the 2.4.0 release. We deem it
high priority to upgrade since it is needed to support PostgreSQL 12.8
and 13.4.

**Bugfixes**
* timescale#3430 Fix havingqual processing for continuous aggregates
* timescale#3468 Disable tests by default if tools are not found
* timescale#3468 Fix crash while tracking alter table commands
* timescale#3494 Improve error message when adding data nodes
* timescale#3498 Fix continuous agg bgw job failure for PG 12.8 and 13.4

**Thanks**
* @brianbenns for reporting a segfault with continuous aggregates
mkindahl added a commit to mkindahl/timescaledb that referenced this pull request Aug 19, 2021
This release contains bug fixes since the 2.4.0 release. We deem it
high priority to upgrade since it is needed to support PostgreSQL 12.8
and 13.4.

**Bugfixes**
* timescale#3430 Fix havingqual processing for continuous aggregates
* timescale#3468 Disable tests by default if tools are not found
* timescale#3468 Fix crash while tracking alter table commands
* timescale#3494 Improve error message when adding data nodes
* timescale#3498 Fix continuous agg bgw job failure for PG 12.8 and 13.4

**Thanks**
* @brianbenns for reporting a segfault with continuous aggregates
mkindahl added a commit to mkindahl/timescaledb that referenced this pull request Aug 19, 2021
This release contains bug fixes since the 2.4.0 release. We deem it
high priority to upgrade since it is needed to support PostgreSQL 12.8
and 13.4.

**Bugfixes**
* timescale#3430 Fix havingqual processing for continuous aggregates
* timescale#3468 Disable tests by default if tools are not found
* timescale#3468 Fix crash while tracking alter table commands
* timescale#3494 Improve error message when adding data nodes
* timescale#3498 Fix continuous agg bgw job failure for PG 12.8 and 13.4

**Thanks**
* @brianbenns for reporting a segfault with continuous aggregates
mkindahl added a commit to mkindahl/timescaledb that referenced this pull request Aug 19, 2021
This release contains bug fixes since the 2.4.0 release.  We deem it
high priority to upgrade.

The release fixes continous aggregate refresh for postgres 12.8 and
13.4, a crash with ALTER TABLE commands and a crash with continuous
aggregates with HAVING clause.

**Bugfixes**
* timescale#3430 Fix havingqual processing for continuous aggregates
* timescale#3468 Disable tests by default if tools are not found
* timescale#3462 Fix crash while tracking alter table commands
* timescale#3489 Fix continuous agg bgw job failure for PG 12.8 and 13.4
* timescale#3494 Improve error message when adding data nodes

**Thanks**
* @brianbenns for reporting a segfault with continuous aggregates
* @brianbenns for reporting a segfault with continuous aggregates
* @usego for reporting an issue with continuous aggregate refresh on PG 13.4
mkindahl added a commit that referenced this pull request Aug 19, 2021
This release contains bug fixes since the 2.4.0 release.  We deem it
high priority to upgrade.

The release fixes continous aggregate refresh for postgres 12.8 and
13.4, a crash with ALTER TABLE commands and a crash with continuous
aggregates with HAVING clause.

**Bugfixes**
* #3430 Fix havingqual processing for continuous aggregates
* #3468 Disable tests by default if tools are not found
* #3462 Fix crash while tracking alter table commands
* #3489 Fix continuous agg bgw job failure for PG 12.8 and 13.4
* #3494 Improve error message when adding data nodes

**Thanks**
* @brianbenns for reporting a segfault with continuous aggregates
* @brianbenns for reporting a segfault with continuous aggregates
* @usego for reporting an issue with continuous aggregate refresh on PG 13.4
mkindahl added a commit to mkindahl/timescaledb that referenced this pull request Aug 19, 2021
This release contains bug fixes since the 2.4.0 release.  We deem it
high priority to upgrade.

The release fixes continous aggregate refresh for postgres 12.8 and
13.4, a crash with ALTER TABLE commands and a crash with continuous
aggregates with HAVING clause.

**Bugfixes**
* timescale#3430 Fix havingqual processing for continuous aggregates
* timescale#3468 Disable tests by default if tools are not found
* timescale#3462 Fix crash while tracking alter table commands
* timescale#3489 Fix continuous agg bgw job failure for PG 12.8 and 13.4
* timescale#3494 Improve error message when adding data nodes

**Thanks**
* @brianbenns for reporting a segfault with continuous aggregates
* @brianbenns for reporting a segfault with continuous aggregates
* @usego for reporting an issue with continuous aggregate refresh on PG 13.4
mkindahl added a commit that referenced this pull request Aug 19, 2021
This release contains bug fixes since the 2.4.0 release.  We deem it
high priority to upgrade.

The release fixes continous aggregate refresh for postgres 12.8 and
13.4, a crash with ALTER TABLE commands and a crash with continuous
aggregates with HAVING clause.

**Bugfixes**
* #3430 Fix havingqual processing for continuous aggregates
* #3468 Disable tests by default if tools are not found
* #3462 Fix crash while tracking alter table commands
* #3489 Fix continuous agg bgw job failure for PG 12.8 and 13.4
* #3494 Improve error message when adding data nodes

**Thanks**
* @brianbenns for reporting a segfault with continuous aggregates
* @usego for reporting an issue with continuous aggregate refresh on PG 13.4
mkindahl added a commit that referenced this pull request Aug 19, 2021
This release contains bug fixes since the 2.4.0 release.  We deem it
high priority to upgrade.

The release fixes continous aggregate refresh for postgres 12.8 and
13.4, a crash with ALTER TABLE commands and a crash with continuous
aggregates with HAVING clause.

**Bugfixes**
* #3430 Fix havingqual processing for continuous aggregates
* #3468 Disable tests by default if tools are not found
* #3462 Fix crash while tracking alter table commands
* #3489 Fix continuous agg bgw job failure for PG 12.8 and 13.4
* #3494 Improve error message when adding data nodes

**Thanks**
* @brianbenns for reporting a segfault with continuous aggregates
* @brianbenns for reporting a segfault with continuous aggregates
* @usego for reporting an issue with continuous aggregate refresh on PG 13.4
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.

None yet

3 participants