search

xeqi / kerodon

interaction and testing library for html based ring apps.
304 stars 22 forks source link

Adding text/regex-in?, link?, heading? predicates. #19

Closed tokenshift closed 9 years ago

tokenshift commented 10 years ago

Added text-in? predicate.

Added regex-in? predicate.

Added link? predicate.

Added heading? predicate.

Updating readme.

glenjamin commented 10 years ago

Seems mostly sensible.

There's a stray println on line 72 Should we be able to match a link on href and text?

I'm not sure whether we should keep adding more validate macros, or if we should try and go down the route of #8, happy to merge this with the above fixed though.

tokenshift commented 10 years ago

Links: I'd like to be able to check both for a link with the expected text or a link with the expected target, depending on the test case (e.g. there must be a link to the login page, but I don't care whether it says "Login" "Log In" or "Sign In"). It might be better to make this explicit, though; e.g. default to href, but pass an option to link? to check link text instead.

Line 72: Thanks, sorry I missed that.

I agree that #8 looks like a good idea, and I think it would make validators like these redundant. I'll fix the stray println, but it's probably not worth it to merge this if it looks like #8 is going to happen.

glenjamin commented 10 years ago

8 is a way off yet.

I think link href assert is useful, and link href + text is useful, but unsure when you'd do just text?

Could we just make text an optional second argument?

On 4 Dec 2013, at 14:13, tokenshift notifications@github.com wrote:

Links: I'd like to be able to check both for a link with the expected text or a link with the expected target, depending on the test case (e.g. there must be a link to the login page, but I don't care whether it says "Login" "Log In" or "Sign In"). It might be better to make this explicit, though; e.g. default to href, but pass an option to link? to check link text instead.

Line 72: Thanks, sorry I missed that.

I agree that #8 looks like a good idea, and I think it would make validators like these redundant. I'll fix the stray println, but it's probably not worth it to merge this if it looks like #8 is going to happen.

— Reply to this email directly or view it on GitHub.

tokenshift commented 10 years ago

I'd test for just the link text if the target page doesn't exist yet, and I don't know what the URL will be, only that there will be a link that says "foo". I'll see if I can make that a configurable thing, though; it definitely makes sense to have href be the default.

glenjamin commented 10 years ago

Hrm, from a BDD point of view I'd pick that moment to decide on what the URL should be - can always update the test if it needs to change.

On 4 Dec 2013, at 14:20, tokenshift notifications@github.com wrote:

I'd test for just the link text if the target page doesn't exist yet, and I don't know what the URL will be, only that there will be a link that says "foo". I'll see if I can make that a configurable thing, though; it definitely makes sense to have href be the default.

— Reply to this email directly or view it on GitHub.

xeqi commented 10 years ago

I'm not sure whether we should keep adding more validate macros, or if we should try and go down the route of #8, happy to merge this with the above fixed though.

I'm fine with adding more validate macros for now.

I'd test for just the link text if the target page doesn't exist yet, and I don't know what the URL will be, only that there will be a link that says "foo".

I prefer to use kerodon for user level tests that look like a real interaction, so I'd use click for that purpose. I can see wanting to find the url+href matching, so perhaps using an optional arg for href could work.

tokenshift commented 10 years ago

Modified it to match href by default, but you can pass :href or :text as the first parameter to control the behavior.

(link? "/foo")
; or
(link? :href "/foo")
; to match href,
(link? :text "Foo")
; to match text.
tokenshift commented 10 years ago

If I'm testing the actual login functionality (what happens when a user clicks login?) I would definitely use URLs and 'click' instead of checking for link presence. However, there are cases where I want to check for the existence of a link or some text as a post-condition of some previous action, without actually interacting with it (e.g. when I sign in or go to the home page as such-and-such user, I should see a "Welcome" message with an "About" link).

I agree that checking for href AND text is more likely than just text, but I didn't want to rule out the latter entirely by forcing you to specify an href if you want to check the link text.

tokenshift commented 10 years ago

Sorry, just realized I was committing with my work email rather than my GitHub email. (= "tokenshift" "Nathan Clark")

jayp commented 9 years ago

This pull request seems to have atrophied away.

Until someone does #8, it maybe worthwhile to check this in. regex-in? is much better than how one has to practically use regex? today -- see #28.

glenjamin commented 9 years ago

I'll pick this up in the next few days - i'm going to tweak the tests a bit, but aside from that I'll get this merged in.

Cheers for the bump :)

jayp commented 9 years ago

Looking forward to it.

glenjamin commented 9 years ago

@jayp I've made those changes and published a 0.6.0-SNAPSHOT.

(has (text-in? "text")) didn't read very well as a sentence, so I've renamed to (has (some-text? "text")) instead.

jayp commented 9 years ago

Thanks @glenjamin, and patch contributor @tokenshift. Great work! Makes the library more usable!

xeqi commented 9 years ago

:+1: to @tokenshift and @glenjamin.

strika commented 9 years ago

This is great, but I have to say that I find confusing that both (has (text? "John Doe")) and (has (some-text? "John Doe")) exist. Personally, I find little excuse for text? to exist. And my first thought was that text? supports multi-line strings. Is there a reason to have both?

jayp commented 9 years ago

I can think of two reasons:

  1. current users of text?
  2. performance of text? should be way better for those who really need a full match.
glenjamin commented 9 years ago

I think text? makes the most sense when used with within to check the exact value of a specific element.

Consider (contrived example):

<span class="warning">You could not be logged in</span>
(has (some-text? "logged in"))
strika commented 9 years ago

@glenjamin These kind of errors will happen with some-text?, since we have it, anyway. It will probably become much more commonly used than text?.

@jayp

  1. current users of text? would hardly be affected. They are using text? in combination with within and if text? is changed to work for multi line strings, test would work as before.
  2. performance is an issue, perhaps. But, when comparing performance it's good to include within + text? vs some-text?, as that's probably the most common example.