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

Fix system_queries test to actually test the problem #7613

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

Green-Chan
Copy link
Contributor

The test added in #7604 doesn't reach the HasRangeTableRef function and thus doesn't test what it should.

@JelteF JelteF requested a review from gurkanindibay June 4, 2024 09:32
@JelteF JelteF changed the base branch from release-12.1 to main June 4, 2024 09:33
@JelteF JelteF changed the base branch from main to release-12.1 June 4, 2024 09:34
@JelteF
Copy link
Contributor

JelteF commented Jun 4, 2024

Could you change the base for this PR to the main? We normally merge everything into main and then cherry-pick to the release branches, but we didn't follow that approach by accident for #7604

@Green-Chan Green-Chan changed the base branch from release-12.1 to main June 5, 2024 09:18
Copy link
Contributor

@gurkanindibay gurkanindibay left a comment

Choose a reason for hiding this comment

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

Thanks @Green-Chan for the fix.

@JelteF JelteF enabled auto-merge (squash) June 27, 2024 14:08
auto-merge was automatically disabled July 16, 2024 06:57

Head branch was pushed to by a user without write access

@Green-Chan
Copy link
Contributor Author

I've rebased the branch to updated main.
Check if all GUCs are sorted alphabetically has failed, and I can't see how since shared_library_init.c isn't even changing in this PR.
Could someone please help me?

Copy link

codecov bot commented Jul 16, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 73.34%. Comparing base (3c467e6) to head (1db28fa).

❗ There is a different number of reports uploaded between BASE (3c467e6) and HEAD (1db28fa). Click for more details.

HEAD has 75 uploads less than BASE
Flag BASE (3c467e6) HEAD (1db28fa)
16_regress_check-pytest 1 0
15_regress_check-pytest 1 0
15_regress_check-follower-cluster 1 0
16_regress_check-query-generator 1 0
14_regress_check-enterprise-failure 1 0
14_16_upgrade 1 0
14_15_upgrade 1 0
15_16_upgrade 1 0
15_regress_check-columnar-isolation 1 0
16_regress_check-split 1 0
16_regress_check-enterprise-failure 1 0
16_regress_check-enterprise-isolation-logicalrep-2 1 0
14_regress_check-enterprise-isolation-logicalrep-1 1 0
14_regress_check-enterprise-isolation 1 0
15_regress_check-enterprise-isolation-logicalrep-1 1 0
15_regress_check-columnar 1 0
15_regress_check-enterprise-failure 1 0
15_regress_check-operations 1 0
16_regress_check-enterprise-isolation-logicalrep-1 1 0
16_regress_check-vanilla 1 0
15_regress_check-multi-mx 1 0
16_regress_check-failure 1 0
16_regress_check-enterprise-isolation 1 0
15_regress_check-failure 1 0
14_regress_check-multi-mx 1 0
_upgrade 17 3
14_regress_check-vanilla 1 0
16_regress_check-enterprise 1 0
15_regress_check-enterprise 1 0
15_regress_check-query-generator 1 0
15_regress_check-vanilla 1 0
14_regress_check-split 1 0
14_regress_check-enterprise 1 0
14_regress_check-operations 1 0
16_regress_check-multi-1 1 0
15_regress_check-multi-1 1 0
14_regress_check-multi-1 1 0
16_regress_check-operations 1 0
15_regress_check-enterprise-isolation 1 0
14_regress_check-query-generator 1 0
14_regress_check-columnar-isolation 1 0
14_regress_check-follower-cluster 1 0
15_regress_check-enterprise-isolation-logicalrep-3 1 0
14_regress_check-enterprise-isolation-logicalrep-3 1 0
16_regress_check-enterprise-isolation-logicalrep-3 1 0
14_regress_check-columnar 1 0
14_regress_check-enterprise-isolation-logicalrep-2 1 0
15_regress_check-enterprise-isolation-logicalrep-2 1 0
16_regress_check-columnar 1 0
15_regress_check-split 1 0
15_regress_check-multi 1 0
14_regress_check-isolation 1 0
15_regress_check-isolation 1 0
14_regress_check-failure 1 0
16_cdc_installcheck 1 0
15_cdc_installcheck 1 0
16_regress_check-multi-mx 1 0
16_regress_check-isolation 1 0
14_regress_check-multi 1 0
16_regress_check-columnar-isolation 1 0
14_regress_check-pytest 1 0
16_regress_check-follower-cluster 1 0
Additional details and impacted files
@@             Coverage Diff             @@
##             main    #7613       +/-   ##
===========================================
- Coverage   89.70%   73.34%   -16.37%     
===========================================
  Files         283      283               
  Lines       60506    60063      -443     
  Branches     7539     7454       -85     
===========================================
- Hits        54276    44051    -10225     
- Misses       4073    13162     +9089     
- Partials     2157     2850      +693     

@Green-Chan
Copy link
Contributor Author

Finally all required tests have passed!
@JelteF could you please merge it?
I would also love some advice for the future how to deal with randomly failing required tests. I can't rerun them other than by pushing something new to the branch, but pushing disables auto-merge.

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