Skip to content

Conversation

@mciesielski2
Copy link

@atlassian-cla-bot
Copy link

Thank you for your submission! Like many open source projects, we ask that you sign our CLA (Contributor License Agreement) before we can accept your contribution.
If your email is listed below, please ensure that you sign the CLA with the same email address.

The following users still need to sign our CLA:
❌mciesielski2

Already signed the CLA? To re-check, try refreshing the page.

),
actionMeter = ActionMeter.Builder(AppendableActionMetricOutput(segment.actionOutput))
.virtualUser(segment.id)
.appendPostMetricHook(DrillDownHook(JavascriptW3cPerformanceTimeline(segment.driver.getDriver())))
Copy link
Author

Choose a reason for hiding this comment

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

The meaningful change.

Copy link
Contributor

Choose a reason for hiding this comment

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

You didn't need that for testing. Any Scenario (e.g. one in jira-actions module or in any other repo) can override the ActionMeter passed onto its actions. And you can create a modified copy of any ActionMeter via its Builder

Copy link
Author

Choose a reason for hiding this comment

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

Oh, didn't think about that, I should have asked earlier x)
So where do you think should the hook be placed finally - here or in the com.atlassian.jira.test.performance.scenario.AllActionScenario?

Copy link
Contributor

@dagguh dagguh Jul 2, 2021

Choose a reason for hiding this comment

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

The ActionMeter already has default postMetricHooks as an empty list. The JavascriptW3cPerformanceTimeline should remain opt-in, because it has a big storage and transfer overhead. So basically, it should not be put in to any JPT API.

The end-consumers like our internal regression tests or hardware recommendations can opt-in if they want to. You already opted-in the AbstractJiraCoreScenario (just tests, not API) in jira-actions, so it got covered by the JiraCoreScenarioIT integration test. 👌
PS. now I also see that there's a dedicated JiraCoreScenarioDevloop in there for fast local devloop.

Copy link
Contributor

Choose a reason for hiding this comment

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

In general, each JPT lib should be self-sufficient in terms of devloop. There's a high chance it already has the necessary testing harness. If not, it should be added/extended.

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.

2 participants