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

add hooks parameter to vernier #295

Merged
merged 3 commits into from
May 23, 2024

Conversation

lHydra
Copy link
Contributor

@lHydra lHydra commented May 18, 2024

What is the purpose of this pull request?

Add hooks configuration parameter for TestProf::Vernier to add event markers from Active Support Notifications

Checklist

  • I've added tests for this change
  • I've added a Changelog entry
  • I've updated a documentation

Closes #289

Copy link
Collaborator

@palkan palkan left a comment

Choose a reason for hiding this comment

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

Thanks!

Let's try to add a test? At least a smoke test (simply checking that when the hook is provided we do not fail). We can also try to parse the report file and check if it contains some ActiveSupport event; though not sure if it's doable.


def initialize
@mode = ENV.fetch("TEST_VERNIER_MODE", :wall).to_sym
@target = (ENV["TEST_VERNIER"] == "boot") ? :boot : :suite

sample_interval = ENV["TEST_VERNIER_INTERVAL"].to_i
@interval = (sample_interval > 0) ? sample_interval : nil
@hooks = (ENV["TEST_VERNIER_HOOKS"] == "rails") ? :rails : nil
Copy link
Collaborator

Choose a reason for hiding this comment

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

We should probably take into account possible future named hooks; so, having smith like: ENV["TEST_VERNIER_HOOKS"]&.split(",").map(&:to_sym) would be better.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

@lHydra
Copy link
Contributor Author

lHydra commented May 22, 2024

Thanks!

Let's try to add a test? At least a smoke test (simply checking that when the hook is provided we do not fail). We can also try to parse the report file and check if it contains some ActiveSupport event; though not sure if it's doable.

Report contains markerSchema, which includes rails events when the hook is enabled. So in the test, I decided to check for a rails event

@lHydra lHydra requested a review from palkan May 22, 2024 17:46
Copy link
Collaborator

@palkan palkan left a comment

Choose a reason for hiding this comment

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

🚀

@palkan palkan merged commit 29e6ac8 into test-prof:master May 23, 2024
17 checks passed
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.

Add Rails/ActiveSupport hooks support for Vernier profiles
2 participants