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

Made TestHelpers include methods from WaitForLoaded #3325

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

Conversation

iainbeeston
Copy link
Contributor

@iainbeeston iainbeeston commented Oct 10, 2024

Description

Right now many of the test helpers don't work in any other project because they rely on methods from WaitForLoaded and the test helpers crash when they are called. This isn't an issue in the avo project itself because the WaitForLoaded methods are required automatically by the avo test suite. It isn't possible to load WaitForLoaded in projects using the rubygem of avo because files inside the spec directory aren't included (and even if they were it would be a bit weird to load something from spec/support in a gem).

To fix this I've moved WaitForLoaded alongside the TestHelpers module and included it from there so the helper methods can be used in other projects.

Fixes # (issue)

Checklist:

  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • I have added tests that prove my fix is effective or that my feature works

Screenshots & recording

Manual review steps

  1. Confirm avo test suite still works correctly
  2. Check test helpers (eg. add_tag) work in a project using avo

Manual reviewer: please leave a comment with output from the test if that's the case.

wait_for_element_missing("turbo-frame[src='#{frame_src}'][busy]", time)
end

def wait_for_body_class_missing(klass = "turbo-loading", time = Capybara.default_max_wait_time)
Copy link

Choose a reason for hiding this comment

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

Method wait_for_body_class_missing has a Cognitive Complexity of 15 (exceeds 5 allowed). Consider refactoring.

puts "\n\nMethod '#{__method__}' raised 'Timeout::Error' after #{time}s"
end

def wait_for_element_missing(identifier = ".element", time = Capybara.default_max_wait_time)
Copy link

Choose a reason for hiding this comment

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

Method wait_for_element_missing has a Cognitive Complexity of 7 (exceeds 5 allowed). Consider refactoring.

end
end

def wait_for_element_present(identifier = ".element", time = Capybara.default_max_wait_time)
Copy link

Choose a reason for hiding this comment

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

Method wait_for_element_present has a Cognitive Complexity of 7 (exceeds 5 allowed). Consider refactoring.

Copy link

codeclimate bot commented Oct 10, 2024

Code Climate has analyzed commit 29ace07 and detected 3 issues on this pull request.

Here's the issue category breakdown:

Category Count
Complexity 3

View more on Code Climate.

@iainbeeston iainbeeston force-pushed the allow-test-helpers-to-be-reused branch from b139358 to 79ea9fb Compare October 10, 2024 12:01
Right now many of the test helpers don't work in any other project because they rely on methods from WaitForLoaded and the test helpers crash when they are called. This isn't an issue in the avo project itself because the WaitForLoaded methods are required automatically by the avo test suite. It isn't possible to load WaitForLoaded in projects using the rubygem of avo because files inside the spec directory aren't included (and even if they were it would be a bit weird to load something from `spec/support` in a gem).

To fix this I've moved WaitForLoaded alongside the TestHelpers module and included it from there so the helper methods can be used in other projects.
@iainbeeston iainbeeston force-pushed the allow-test-helpers-to-be-reused branch from 79ea9fb to 29ace07 Compare October 10, 2024 13:21
@iainbeeston
Copy link
Contributor Author

I haven't fixed any of the code climate comments because I didn't want to change that code.

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.

1 participant