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

Remind users to assert for unique content on each new page #46

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

coolprobn
Copy link

Closes #44

Tasks

  • Modal to show reminder to assert for unique content on each new page

Issue with default alert

When subscribing to on page load event and trying to show normal alert box from the browser it throws the error:

Error:
BasicsTest#test_getting_started:
Selenium::WebDriver::Error::UnexpectedAlertOpenError: unexpected alert open: {Alert text : this breaks the page}
  (Session info: chrome=98.0.4758.109)
    test/application_system_test_case.rb:243:in `execute'
    test/system/basics_test.rb:7:in `block in <class:BasicsTest>'

Due to this, I have implemented a simple JS and CSS modal for this feature instead of using alert.

UI Screenshot

Assertion Modal

Closes #44

Tasks
---

- Modal to show reminder to assert for unique content on each new page
@andrewculver
Copy link
Contributor

andrewculver commented Mar 4, 2022

@coolprobn This is close, but there's a big issue: The JS fires the reminder even during normal running of system tests, which breaks the entire Bullet Train test suite. Instead, we need to make it so that this new reminder feature is only activated after the test encounters the magic_test command, and it is deactivated right after the user concludes their magic_test session, e.g. in lib/magic_test/support.rb:

    def magic_test
      empty_cache
      @test_lines_written = 0
      begin
        # 👋 Do something here to activate the notification JS.
        magic_test_pry_hook
        binding.pry
        # 👋 Do something here to disable the notification JS.
      rescue
        retry
      end
    end

You might be able to take advantage of page.evaluate_script in this context to set some variable on window (e.g. window.active_magic_test_session = true) and then check for that before triggering the alert... and then do the same, but = false to disable it. (You can see other examples of page.evaluate_script in the same file.)

@andrewculver
Copy link
Contributor

@coolprobn Also, please make sure the Bullet Train test suite is passing when linked to this branch for it's magic_test dependency in the Gemfile.

@coolprobn
Copy link
Author

@andrewculver I should have checked and tested properly, sorry.

I will fix as required and push new changes soon. Thanks.

@coolprobn
Copy link
Author

coolprobn commented Mar 9, 2022

@andrewculver I have added the following code to reload the page before we stop the system test with magic test:

# reloading the page so that activeMagicTestSession variable is set and recognized by the JS
      # activeMagicTestSession is not set in the beginning because HTML file would have already been rendered before this method is invoked
      page.evaluate_script("window.location.reload()")

This was added because new variable that we are using to know if we are currently in magic test session was not being picked up. It is not recognized without page reload because all HTML and JS Scripts are rendered before this new variable is set inside support.rb

UX hasn't changed and there is a very minimum chance that user know what happened. I would love to hear your thoughts if there are any other solutions on your mind to resolve this issue.

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.

Remind users to assert for content on each new page.
3 participants