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 helper methods for starting/stopping trace #66

Merged
merged 1 commit into from
Nov 13, 2023

Conversation

yuki24
Copy link
Contributor

@yuki24 yuki24 commented Nov 10, 2023

Given the issue #65, it seems like there is some general interest in a simpler interface for using traces. The code in this PR is the code I use to test https://toprubycompanies.info in CI, and it's been working very well.

With the new interfaces introduced by this PR, using Playwright's trace is as simple as:

class ApplicationSystemTestCase < ActionDispatch::SystemTestCase
  driven_by :playwright

  setup do
    page.driver.start_tracing(title: method_name) if retrying?
  end

  def before_teardown
    if retrying?
      page.driver.stop_tracing(path: "./tmp/playwright/traces/#{method_name}.zip"))
    end
  ensure
    super
  end

  def retrying?
    # Determine whether the run is a retry...
  end
end

With RSpec, it could look like:

around :example do |example|
  if should_trace?
    page.driver.trace path: "./tmp/playwright/traces/#{example.metadata[:full_description]}.zip" do
      example.run
    end
  end
end

def should_trace?
  # Somehow determine whether the run should be traced.
end

@YusukeIwaki
Copy link
Owner

Thank you for your contribution.

it seems like there is some general interest in a simpler interface for using traces.

After several investigation, I completely agree with this.

end

# Trace execution of the given block. The tracing is automatically stopped when the block is finished.
def trace(name: nil, screenshots: false, snapshots: false, sources: false, title: nil, path: nil, &block)
Copy link
Owner

@YusukeIwaki YusukeIwaki Nov 12, 2023

Choose a reason for hiding this comment

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

@yuki24 I prefer the method name with_tracing for this. Do you have any ideas about this?

Copy link
Contributor Author

@yuki24 yuki24 Nov 13, 2023

Choose a reason for hiding this comment

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

I think the name with implies that the word tracing is a thing, rather than being an action. For example, all the methods prefixed with with_ in Rails follow the form of with_thing, rather than with_doing:

スクリーンショット 2023-11-13 10 31 33

Whereas other methods, such as #draw, #transaction, and #freeze_time, all trigger an explicit action under the hood, and the execution in the block is in effect of the action you would like to take. I believe this case is closer to the latter.

So based on this assessment, I would think that the name #trace is more intuitive than #with_tracing.

With that all said, this is ultimately your call. Let me know what you think.

Copy link
Owner

Choose a reason for hiding this comment

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

Thank you for your kind advices.
I didn't know well about the Rails existing method names and its rules, and your advices really helped me.

Let's keep the method name 'with_trace' here.

@YusukeIwaki YusukeIwaki merged commit d3f27e0 into YusukeIwaki:main Nov 13, 2023
8 of 9 checks passed
@yuki24
Copy link
Contributor Author

yuki24 commented Nov 14, 2023

Thank you!

@yuki24 yuki24 deleted the add-tracing-helpers branch November 14, 2023 13:17
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