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

PMM-12468 Plan summary, COLLSCAN. #920

Merged
merged 2 commits into from
Feb 6, 2025

Conversation

JiriCtvrtka
Copy link
Contributor

@JiriCtvrtka JiriCtvrtka commented Jan 29, 2025

Ticket on PMM side: https://perconadev.atlassian.net/browse/PMM-12468

  • The contributed code is licensed under GPL v2.0
  • Contributor Licence Agreement (CLA) is signed
  • util/update-modules has been ran
  • Documentation updated
  • Test suite update

@it-percona-cla
Copy link

it-percona-cla commented Jan 29, 2025

CLA assistant check
All committers have signed the CLA.

Copy link
Collaborator

@svetasmirnova svetasmirnova left a comment

Choose a reason for hiding this comment

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

Please add regression tests.

@BupycHuk
Copy link
Member

@svetasmirnova where can we find CI tests execution results?

@svetasmirnova
Copy link
Collaborator

@BupycHuk

I expect you to add tests to src/go/mongolib/proto/system.profile_test.go and src/go/mongolib/stats/stats_test.go and run those tests locally when change code.

Proper testing for Go tools is planned but not done yet.

@it-percona it-percona temporarily deployed to PMM-12468-plan-summary-and-coll-scan - percona-toolkit PR #920 February 3, 2025 16:24 — with Render Destroyed
@JiriCtvrtka
Copy link
Contributor Author

JiriCtvrtka commented Feb 3, 2025

@svetasmirnova

The stats_test.go tests are currently completely skipped (even locally).
Check skip here
I re-enabled them in this PR, but they are failing even without any changes. So they have probably been broken for a while.

The system.profile_test.go tests, which check explain and my changes about plan summary, are not related to it.

Due to the issues mentioned above, I created a ticket:
PMM-13736
to address these problems (failing tests and possible incorrect BSON tags in system.profile lock properties).

Additionally, I corrected some other tests, such as:
Check example of corrected cleanup,
where the previous code could cause incorrect cleanup after tests (direct exit will ignore defer functions).

Would you agree to merge this PR and cover my changes with tests in the ticket, where we will fix the stats tests and related issues together?

@JiriCtvrtka JiriCtvrtka marked this pull request as ready for review February 6, 2025 15:06
Copy link
Collaborator

@svetasmirnova svetasmirnova left a comment

Choose a reason for hiding this comment

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

OK, approving it now. Let's fix tests in https://perconadev.atlassian.net/browse/PT-2423

@svetasmirnova svetasmirnova merged commit 75cddb4 into 3.x Feb 6, 2025
6 checks passed
@svetasmirnova svetasmirnova deleted the PMM-12468-plan-summary-and-coll-scan branch February 6, 2025 15:34
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.

5 participants