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

Make the 5 second timeout on test-running specifiable by an optional parameter to phantom.test.add #5

Merged
merged 3 commits into from
Sep 2, 2011

Conversation

harob
Copy link
Collaborator

@harob harob commented Sep 1, 2011

Note that because this is an optional parameter it should not break any existing tests.
Also, this makes phantom.test.add mirror @body.assertFirst, as both now optionally take the optional argument total.

Please let me know if there are any edits you would like me to make!

@joshbuddy
Copy link
Owner

A couple of questions about this .. one, where is it needed? This timer gets destroy and recreated between each assertion. I'm wondering where you've bumped against it. Two, wanna re-merge and write some tests? Thank you!

@harob
Copy link
Collaborator Author

harob commented Sep 2, 2011

I need this functionality to test a page with a Flash application that takes a very long time (~10 seconds) to initialize.

So my test looks something like the following:

phantom.test.add "Long running test", total: 20, ->
  @get '/page_with_flash_app', ->
    @wait 15, (->
      @body.assertFirst '#flashApp', (f) ->
        f.someFlashAppAPIMethod() == "expectedValue"
      @succeed()
    )

If I missing some existing functionality that already accomplishes this, please let me know!

Regarding adding tests: It looks like the /test directory contains sample apps with sample tests; so should I just add a sample test that uses this new functionality? Or is there some automated unit testing of the framework that I am missing?

@joshbuddy
Copy link
Owner

Lemme clean up the how-to on testing here. Go ahead and re-merge from master so that this merges cleanly.

@harob
Copy link
Collaborator Author

harob commented Sep 2, 2011

I merged upstream, so this should presumably now merge cleanly. Please me know when you want me to update the tests.

Great work on this whole framework by the way!

@@ -16,7 +16,7 @@ phantom.test.add "Simple form with wait", ->
@wait 1, ->
@succeed()

phantom.test.add "Slow form", ->
Copy link
Owner

Choose a reason for hiding this comment

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

Why change this test? Shouldn't you add a different test for this? Also, can you add the negative test case to non_working_ghost?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Will do.

joshbuddy added a commit that referenced this pull request Sep 2, 2011
Make the 5 second timeout on test-running specifiable by an optional parameter to `phantom.test.add`
@joshbuddy joshbuddy merged commit 75d8475 into joshbuddy:master Sep 2, 2011
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