ONEARMY / community-platform

A platform to build useful communities that aim to tackle global problems
https://platform.onearmy.earth
MIT License
1.1k stars 370 forks source link

[e2e] Discussion being tested three times #3766

Closed tuliobluz closed 1 month ago

tuliobluz commented 1 month ago

Is your feature request related to a problem? Please describe.

The discussion functionality is shared by HowTo, Question, and Research, with that, there are e2e to all those 3 features. So, the same functionality is being tested three times. I did not see any big difference among the assertions.

Describe the solution you'd like

The solution is the discussion functionality should be covered by only one spec e2e. Then, we avoid duplicated code, reduce the e2e total run time, and it would be easy to maintain.

You can also check if the discussion is displayed on the other pages.

Describe alternatives you've considered

If it is indispensable to test discussion for the 3 pages, we can do a refactor creating commandsUi to be shared among the 3 specs files. Then, we avoid duplicated code and it would be easy to maintain.

benfurber commented 1 month ago

While the user action is currently identical across the three document types, the following is different:

(This is especially true of research compared to the other two.) So without the seperate specs, it would be very easy to create regressions.

I agree the code is repeative though, so I like the idea of a command ui or two.

mariojsnunes commented 1 month ago
  • the templating with business logic
  • the side effects through the discussion store
  • some outputs have their own independent business logic

Would it be possible to reduce/eliminate those differences?

tuliobluz commented 1 month ago

Another alternative to cover the Business is using API testing via Cypress.

I was trying to go over the code, but I am not 100% sure, those are the parts that have differences, and Research is the one where most distinguished from HowTo and Question.

Approach:

benfurber commented 1 month ago

Well there isn't currently an API to test. :)

Start with the commandsUi and go from there?

tuliobluz commented 1 month ago

That is true, I forgot about the API.

We have two options:

  1. Add all possible steps into commndsUi (Already described comments above)
  2. Remove some steps (described below)

Example of Research and Questions to be removed:

Then we leave the following steps (Those here we can convert into commandsUi)

And for howTo we leave all the steps to make the full test of the functionality

benfurber commented 1 month ago

Let's go with 1 please.

benfurber commented 1 month ago

There's also some missing coverage on research discussions which meant #3778 slipped through. Should hide the comments after deletion and check the button text has the right comment count.

tuliobluz commented 1 month ago

@benfurber I created a PR for the first solutions and also added assertions to check the comment count.

tuliobluz commented 1 month ago

Closing the issue as it was merged