[DISCUSS] Create FollowerIndexFormPageObject and apply it to follower_index_add tests

[cjcenizal]

I'm creating this to share some ideas with the team and gather feedback. I wanted to try architecting Seb's test helpers a bit differently, to see if it would yield any benefits.

I was inspired by the Page Object pattern used in functional tests when I explored this direction. In that context, they act as an abstraction layer between your tests and the UI being tested. The intention is to make tests more expressive and less dependent upon HTML-level implementation details (e.g. test subject selectors). I'm not sure if this is an intended benefit but I've also always liked how I can scan the page object and "see" what I'm capable of doing with a particular UI.

After I applied the pattern to our tests, I spotted what I think are three main benefits:

1. OOP paradigm on the test level. In the context of the text suite, the code reads the way you would interact with the page because we're dealing with object-oriented nouns and verbs. There's also less emphasis on the test subject selector values themselves, which I find difficult to scan because they tend to be very verbose and repetitive. In my opinion the end result is increased code readability.
2. Simple functional paradigm for lower-level helper functions. The helper functions don't require currying (not sure if that's the right term here, but it seems like it?) to be useful, which I find makes them easier to understand. Instead of currying initTestBed, initUserActions, registerTestBed, and getUserActions, helpers are pure functions which are only consumed by the page object and not used directly within the tests.
3. Modular and more reusable code. By creating clear boundaries between the page object and the helper functions, we end up with helper functions that stand on their own, uncoupled from larger concepts (such as the testbed). This increases their reusability, e.g. in other tools or unit tests. We could even extract many of them into the EUI test utils module for use outside of Kibana.

@elastic/elastic-ui, @silne30, and Seb, I'd love to hear everyone's thoughts on this.

[elasticmachine]

Pinging @elastic/es-ui

[elasticmachine]

💔 Build Failed

• continuous-integration/kibana-ci/pull-request

[cuff-links]

Hey, CJ. This is pretty interesting. I have never seen a jest style Page Object Model. Same concept but uses the loading of components in place of searching for selectors. I like it. Definitely want to talk to you more in depth about it when I get back.

[sebelga]

Hey @cjcenizal thanks for bringing this.

So I will start with my conclusion 😊 and explain my thoughts from there. To me, this comes all down to a developer preference and code style. As such, I don't think we should impose a pattern to how test helpers should be written and have our focus on writing tests that are easy to read and to reason about (whichever the helpers used).

With that said, I do think that bringing this PR with the page object pattern opens the discussion to see how we could still improve the readability of the current tests or future tests. But I don't think it is necessary to go in a completely different direction to achieve that. And, to repeat, it would only come down to preference at the end.

So.... everything I'll say below this line is my preference and is subjective. 😊

To be able to compare apples with apples I created a "follower_index_form.page_object-v2.js" file that has the same functionality as the one you added but with much fewer lines of code (43 lines vs 83).

1. OOP paradigm
   I am more and more about declarative instead of imperative (again preference). One of the first things we see is that by being declarative, the code is less verbose

And I, personnaly, read it better:

expect(exists('loadingRemoteClusters')).toBe(true);
// vs
expect(followerIndexPage.getLoadingRemoteClusters().length).toBe(1);

And for the "developer experience", it is also (again for me) better. "Need to add a new page object?": add a line in the objectsToDataTestSubject and it's done. No need to create a 3 lines functions.

2. Simple functional paradigm for lower-level helper functions.

With the v2 version, I bring you can see that it comes down to 2 lines. Instead of a registerTestBed we could have a new TestBed(). It's Object programming vs Functions. This is again just a preference.
The complexity you have seen in the current tests is that I have 1 test helper file for all the tests files of the app. Obviously, it brings a bit of complexity in that file. But you might be right and extracting what's unique to each test file to its own helper/page object file, could help in readability.

3. Modular and more reusable code.
   Again, yes creating boundaries and separating general helpers from "page objects and actions" unique to a test file is a great idea and we should do it.

This increases their reusability, e.g. in other tools or unit tests.

I would need more context to understand this, otherwise, I would say "yagni" 😊

---

To conclude I would say: I prefer to give the devs a low-level API with helpers for their testing (the "testBed") and then it is free to them to create a small abstraction and expose page object to their tests if they want. But I would not impose a pattern, and I don't think there is a need to refactor the current tests other than for a preference (although I do think we could improve and create the boundaries like the example I give in "v2").

From this conversation, I am thinking of converting the testBed to Typescript in order to get this

[elasticmachine]

💔 Build Failed

• continuous-integration/kibana-ci/pull-request

[bmcconaghy]

I think I favor the more functional approach here as opposed to the page object one. The only modification I might suggest is to establish constants for the test subject names, but if that can be accomplished as a typescript thing as @sebelga shows in the v2 screenshot, that would be nifty too.

[cjcenizal]

@sebelga It sounds like we agree on a couple things we can move forward with. Let's do this:

1. Extract your super-handy helper functions like getFormErrorsMessages, setInputValue, selectCheckBox, setComboBoxValue, and getMetadataFromEuiTable out of testbed.js and into EUI's test service module. We should add tests to verify behavior. This way they won't be coupled to testbed.js, though once they're extracted out we can refactor testbed.js to consume them.
2. Break up the test helpers files into a single file per test.

How does this sound? Let me know if you'd like help with any of this! I'm closing this issue because it's served its purpose, but if anyone else has thoughts to share feel free to add them here.

[sebelga]

Sounds great @cjcenizal I actually started this work already and hopefully should have a PR by the end of the day. I am not sure though about moving the testbed method to a different repo. I actually moved (for now) the findTestSubject to x-pack.
It seems to me that EUI (as a UI library) should not be maintaining helpers function for our client-side integration tests, as they are meant to be used in the context of an app. And the developer experience to update or add new helpers would be quite bad (open PR in eui, get it merged, make a release of EUI, open PR in Kibana, get it merged...). But let's discuss it over zoom