QA: Discuss change of behavior of element annotation

Based on a discussion started on slack: https://gitlab.slack.com/archives/C3JJET4Q6/p1516206052000403

@brodock: when we define the element, with a pattern we are supposed to reuse it or just annotate there and still write the same code as before? also /form_tag.+id: 'project\-filter\-form'/ can’t really be implemented as: form#project-filter-form ?

@mkozono: actually I feel like element patterns beg for css/xpath, but since we're manually checking view files it's not doable. I am also not totally clear yet on how we use regexp there because it didn't seem to do what I expected

@brodock: also let’s say we use css/xpath, that means we can reuse the selector on the other part of the code, so it acts like a “let” in my example here:

element :form_filter_by_name, 'form#project-filter-form'

page.within(form_filter_by_name) do
#...
end

@brodock: by reading the README, looks like there will be a CI step to check for missing elements, that’s a nice opportunity to remove the qa- anti-pattern as well. So we build a single codebase with the selectors everybody needs. If someone removes something that is being used by QA, it will fail the their build and they can either fix the QA or re-add the classes/ids

@godfat: that CI step is qa:selectors in .gitlab-ci.yml, which is bundle exec bin/qa Test::Sanity::Selectors in the qa/ and it's just checking via plain text or regular expression match, so we can't use CSS selectors nor XPath there

@brodock: if we are just reading the sourcecode than this will also not work, because we may be coupling element with implementation detail

like, instead of writting the element using a string, I could be using a variable or a helper

if I change from one to the other the element thing will stop working, even though final html is still unchanged (edited)

if we are comparing rendered html than it’s a different thing. I don’t think we can do that comparison without making at least emulating a “request spec"

@godfat: i think the current approach for qa:selectors to work well is based on qa- selectors. if we're not doing that, we're going to have hard time

@mkozono: what's wrong with using well-named non-qa-specific selectors? Since we have the qa:selectors spec now, which fails fast

quote from @grzesiek from here: I think that the point is that when someone removes / moves selector that starts with qa- it is immediately obvious that GitLab QA that depends on it, is going to have problems. It also explains why we do have a selector in places where it seems to be completely useless. Then it is in-line with what we do with js-* selectors.

@brodock: I never knew we had js-* ones 😕

@mkozono: fair enough, the js- pattern does work

although I feel that pattern helps most with graceful degredation, which is not a problem we are solving with the qa- selectors

@brodock: The argument behind is that we will reach a moment in the future where we need to use both for js- and qa- and we can have only one id (edited)

so the sourcecode starts depending on qa- or someone makes a css change using qa- because it was already there etc

@mkozono: Yeah I think the tradeoff of adding qa- classes is not worth it in most cases


I tried to get most of what we discussed in context, because there is already a lot of good information there, and we can try to improve it further from here.

Edited by 🤖 GitLab Bot 🤖