search

TestStack / TestStack.Seleno

Seleno helps you write automated UI tests in the right way by implementing Page Objects and Page Components and by reading from and writing to web pages using strongly typed view models.
http://teststack.github.com/TestStack.Seleno/
MIT License
180 stars 60 forks source link

InputHtmlControl and generated id #181

Open mattcowen opened 9 years ago

mattcowen commented 9 years ago

Hi

The application I am working on is overriding the id on input elements. I've noticed that the InputHtmlControl.ValueAs and ReplaceInputValueWith methods are using the id when trying to set the value from the model. My question is, would it not be better to use the name since MVC is name centric when dealing with elements? To assume the id is the same as the name (with full stops converted) could be a problem for many people.

In my case, my inputs have an id in the form "viewmodel_property" whereas the name is just "property". I thought that it might be good to specify a prefix for the id somehow but thinking about it, would it make setting the value on radio buttons and checkboxes easier if it was all done by the name attribute?

mwhelan commented 9 years ago

I'm not so sure about MVC being name centric. We use the Html401IdUtil from System.Web.Mvc.TagBuilder to generate the same IDs that MVC does. We also have the MvcControlIdGenerator, though that supports both ID and Name. I think we want to take a minute and decide whether this is a change we want to make. /cc @robdmoore @MehdiK

mattcowen commented 9 years ago

Would developers usually override the id when needing to implement a jquery widget or something on that particular element? One would not touch the name attribute in this case because they want the MVC model binder to still work. Therefore it would be much safer to guess at the name attribute in the vast majority of cases IMO.

Perhaps a switch could be implemented where the tester can set which mode of operation is used; id or name? I would be happy to implement this but it would be good to hear from Rob and Mehdik on this before I do. Not sure on the ramifications.

MehdiK commented 9 years ago

I agree that could be a useful configuration.

The only issue I see is the HtmlControl class that has some smarts in dealing with missing ids. When we configure the framework to use name instead of id by default, the smarts in that class should change to deal with missing names. Can't think of anything else! cc @robdmoore @mwhelan

robdmoore commented 9 years ago

To be honest, I've always thought it was weird that stuff used id. You don't need an id for a form field, but you do need a name. Name makes way more sense.

Ids still make sense in situations that you add an id to an element (form field or otherwise) for the purposes of identifying it for a UI test, but frankly in those situations you are probably going to use Find.Element(By.Id("the-id")) anyway.

Given that the MVC binding to view model properties is done by name rather than id I think it makes sense to make it use name. Frankly, the current approaches in Seleno to target form fields is confusing since there is like 3 different ways to do things - it probably deserves a review. Whether that should be now or later remains to be seen. The main thing to keep in mind is this is a breaking change (probably) so we just need to treat it carefully.

mwhelan commented 9 years ago

When I started all this I based the approaches on the UI testing chapter in "ASP.Net MVC In Action" and they use IDs in that... :-) So, there's no "good" reason for using IDs. If name makes more sense in MVC then we should move towards that. I agree that consistency is really important and we should try to have one way to do things. I'm pretty keen to get up to v1.0, so the question is whether we want to address it before or after...?

By the way, does ASP.Net WebForms also use name rather than id?

robdmoore commented 9 years ago

It's not that mvc uses names (mvc uses both name and id) it's that web browsers use names so regardless of what you are using (mvc, browsers, Ruby on Rails, php, etc.) you will be using names.

On 2 Nov 2014, at 5:08 pm, Michael Whelan notifications@github.com wrote:

When I started all this I based the approaches on the UI testing chapter in "ASP.Net MVC In Action" and they use IDs in that... :-) So, there's no "good" reason for using IDs. If name makes more sense in MVC then we should move towards that. I agree that consistency is really important and we should try to have one way to do things. I'm pretty keen to get up to v1.0, so the question is whether we want to address it before or after...?

By the way, does ASP.Net WebForms also use name rather than id?

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

mwhelan commented 9 years ago

OK, thanks.

slipdef commented 9 years ago

Either I missed something or the Id/Name selectors you use to locate input elements (within Input method or HtmlControlFor<>(controlId)) are not scoped (see #171 request). Let's say I have 2 forms on my page with similar Ids/Names for inputs. How should I locate inputs with just controlId?

robdmoore commented 9 years ago

If they have the same ID that would actually be invalid html :p ID should always be unique.

If you are searching by name then yes, you probably do need scoping.

On 14 Nov 2014, at 8:11 am, slipdef notifications@github.com wrote:

Either I missed something or the Id/Name selectors you use to locate input elements (within Input method or HtmlControlFor<>(controlId) are not scoped (see #171 request). Let's say I have 2 forms on my page with similar Ids/Names for inputs. How should I locate inputs with just controlId?

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

slipdef commented 9 years ago

Agree and that's the problem. HtmlControlFor<> allows to search by ID only.

robdmoore commented 9 years ago

Actually, until we add scoping searching by id better since (assuming it was done right and ids are unique) there is no ambiguity.

Regardless, I think that's a moot point because as I said above I agree that we should search for fields by name :)

On 14 Nov 2014, at 7:52 pm, slipdef notifications@github.com wrote:

Agree and that's the problem. HtmlControlFor<> allows to search by ID only.

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

mwhelan commented 9 years ago

Just to contradict what I wrote earlier in this issue, that there is no good reason to use ID. Selenium docs says that ID is "the most efficient and preferred way to locate an element." http://docs.seleniumhq.org/docs/03_webdriver.jsp#by-id

I've seen this advice repeated in a few books/websites. The hierarchy of preference is along the lines of ID, Name, CSS, xpath.

How about the idea of searching for ID first, then fall back to searching by name if an ID is not found?

slipdef commented 9 years ago

IMO API for HtmlControlFor<> should be more flexible and support jQuery selector. It's up to a client to decide how they want to search a control, API shouldn't dictate in that particular case.

mattcowen commented 9 years ago

I disagree. I personally love the speed of testing I get from passing a view model instance with all my test data to the Input method call and having Seleno try to work it out. I think that is a great idea and makes Seleno well worth the effort. The time saved in that one call is potentially immense.

Perhaps there are some legs in Michael's idea. I still think some kind of switch between the two might be a good first step so there is some control for the developer.

Matt

On Fri, Nov 14, 2014 at 2:53 PM, slipdef notifications@github.com wrote:

IMO API for HtmlControlFor<> should be more flexible and support jQuery selector. It's up to a client to decide how they want to search a control, API shouldn't dictate in that particular case.

— Reply to this email directly or view it on GitHub https://github.com/TestStack/TestStack.Seleno/issues/181#issuecomment-63074682 .

mattcowen commented 9 years ago

I have almost done this. There are 2 tests failing however. One is to do with IE driver so I'm not too concerned about it. The other one is to do with boolean properties and how they are treated when calling Input.Model(..). At the moment a boolean property is written to a textbox which is not what the form has rendered for this property. Should I adapt this so it defaults to ticking the checkbox if "true"? Or am I missing something from my understanding and this was not done for a good reason?

Here is the result of the failed test in case you don't also see this.

[Info] 'TestStack.Seleno.Configuration.AppConfigurator' Seleno v0.3.2.0, .NET Framework v4.0.30319.18444 [Debug] 'TestStack.Seleno.Configuration.SelenoApplication' Starting Webserver [Debug] 'TestStack.Seleno.Configuration.SelenoApplication' Browsing to base URL

Scenario: Writing a model Given an empty form [Passed] When writing a model [Passed] Then the model should serialise to the same model [Failed] [

NUnit.Framework.AssertionException:   RequiredBool Expected: True But was:  False  at NUnit.Framework.Assert.That(Object actual, IResolveConstraint expression, String message, Object[] args) at NUnit.Framework.Assert.That(Object actual, IResolveConstraint expression, String message) at TestStack.Seleno.AcceptanceTests.Web.Fixtures.ViewModelEqualsConstraint.Matches(Object actualViewModel) in c:\DevProjects\TestStack.Seleno\TestStack.Seleno\src\TestStack.Seleno.AcceptanceTests.Web\Fixtures\ViewModelEqualsConstraint.cs:line 39 at NUnit.Framework.Assert.That(Object actual, IResolveConstraint expression, String message, Object[] args) at NUnit.Framework.Assert.That(Object actual, IResolveConstraint expression) at TestStack.Seleno.AcceptanceTests.Web.Controllers.Form1Controller.ExpectFixtureA(Form1ViewModel vm) in c:\DevProjects\TestStack.Seleno\TestStack.Seleno\src\TestStack.Seleno.AcceptanceTests.Web\Controllers\Form1Controller.cs:line 22
]

robdmoore commented 9 years ago

That test is deliberately faiIling so all good :)

On 15 Feb 2015, at 11:46 pm, Matt Cowen notifications@github.com wrote:

I have almost done this. There are 2 tests failing however. One is to do with IE driver so I'm not too concerned about it. The other one is to do with boolean properties and how they are treated when calling Input.Model(..). At the moment a boolean property is written to a textbox which is not what the form has rendered for this property. Should I adapt this so it defaults to ticking the checkbox if "true"? Or am I missing something from my understanding and this was not done for a good reason?

Here is the result of the failed test in case you don't also see this.

[Info] 'TestStack.Seleno.Configuration.AppConfigurator' Seleno v0.3.2.0, .NET Framework v4.0.30319.18444 [Debug] 'TestStack.Seleno.Configuration.SelenoApplication' Starting Webserver [Debug] 'TestStack.Seleno.Configuration.SelenoApplication' Browsing to base URL

Scenario: Writing a model Given an empty form [Passed] When writing a model [Passed] Then the model should serialise to the same model [Failed] [

NUnit.Framework.AssertionException: RequiredBool Expected: True But was: False at NUnit.Framework.Assert.That(Object actual, IResolveConstraint expression, String message, Object[] args) at NUnit.Framework.Assert.That(Object actual, IResolveConstraint expression, String message) at TestStack.Seleno.AcceptanceTests.Web.Fixtures.ViewModelEqualsConstraint.Matches(Object actualViewModel) in c:\DevProjects\TestStack.Seleno\TestStack.Seleno\src\TestStack.Seleno.AcceptanceTests.Web\Fixtures\ViewModelEqualsConstraint.cs:line 39 at NUnit.Framework.Assert.That(Object actual, IResolveConstraint expression, String message, Object[] args) at NUnit.Framework.Assert.That(Object actual, IResolveConstraint expression) at TestStack.Seleno.AcceptanceTests.Web.Controllers.Form1Controller.ExpectFixtureA(Form1ViewModel vm) in c:\DevProjects\TestStack.Seleno\TestStack.Seleno\src\TestStack.Seleno.AcceptanceTests.Web\Controlle rs\Form1Controller.cs:line 22 ] — Reply to this email directly or view it on GitHub.

mattcowen commented 9 years ago

Ok thanks. Will create a PR in that case.

I'm curious to know the best way to handle a boolean/checkbox when writing the model to the view. Do I blacklist the property and handle manually? Or is there a way of creating an override for writing the model that will allow me to handle it whatever way I decide on?

robdmoore commented 9 years ago

From memory the problem is a list of checkboxes rather than a single one and the problem is the mvc model binder. Can't remember now though :p

On 16 Feb 2015, at 4:01 am, Matt Cowen notifications@github.com wrote:

Ok thanks. Will create a PR in that case.

I'm curious to know the best way to handle a boolean/checkbox when writing the model to the view. Do I blacklist the property and handle manually? Or is there a way of creating an override for writing the model that will allow me to handle it whatever way I decide on?

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