Skip to content

feat: DHIS2-9276 #12 - e2e tests for working lists - #1402

Closed
JoakimSM wants to merge 63 commits into
masterfrom
DHIS2-9276-12-cypress-tests
Closed

feat: DHIS2-9276 #12 - e2e tests for working lists#1402
JoakimSM wants to merge 63 commits into
masterfrom
DHIS2-9276-12-cypress-tests

Conversation

@JoakimSM

@JoakimSM JoakimSM commented Jan 11, 2021

Copy link
Copy Markdown
Member

Cypress tests for tei and single event working lists.

@JoakimSM JoakimSM changed the title feat: DHIS2-9276 #12 - e2e tests feat: DHIS2-9276 #12 - e2e tests for working lists Jan 11, 2021
<Table
className={classes.table}
data-test="dhis2-capture-event-list-table"
data-test="online-list-table"

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Making a change to this data test attribute since this component is used for both events and teis. @vilkg (will be merged to master soon)

</Head>
<Body
data-test="dhis2-capture-event-list-body"
data-test="online-list-body"

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same as above @vilkg

return {
sortById: column.id,
sortByDirection: 'desc',
sortByDirection: getSortDirection(column.type),

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

minor functionality change.

@paschalidi paschalidi left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good thanks for adding the tests Joakim! 🙏

I wanted to say that is good effort that we are trying to follow the text whenever is possible. And you have been doing that here in most cases.

Also wanted to make one comment as a general observation of mine. To me, since these tests are e2e (meaning testing from an end user perspective) seems counter intuitive that we are testing for requests being returned from the api. Users when they navigate through an application they dont know about api responses. They understand if something work by its looks. Generally this is why that article suggested we want to test against text being visible on the screen, because a user interface is broken from an end user perspective when certain views arent being displayed or displayed with wrong information.

Now one question might be, why we wanna these e2e tests to be written from a user perspective? Imagine that we have a migration from an endpoint to another. Lets say we are migrating from an endpoint that used to be /api/events/ to one that is now being renamed to /api/trackerEvents/. Ideally we would like to make the migration and run the tests to see if all the scenarios we used to cover in the e2e tests are still running alright after the migration.

I assume you had good reasons for testing against the api. However you might find useful keeping this perspective I am describing in mind when writing these tests.

Base automatically changed from DHIS2-9276-11-UI to DHIS2-9276-10-container January 18, 2021 10:46
Base automatically changed from DHIS2-9276-10-container to DHIS2-9276-9-filter-button-naming January 18, 2021 10:48
Base automatically changed from DHIS2-9276-9-filter-button-naming to DHIS2-9276-8-hide-empty-menus January 18, 2021 10:48
Base automatically changed from DHIS2-9276-8-hide-empty-menus to DHIS2-9276-disable-modifications January 18, 2021 10:49
Base automatically changed from DHIS2-9276-disable-modifications to DHIS2-9276-custom-template-order January 18, 2021 10:50
Base automatically changed from DHIS2-9276-custom-template-order to DHIS2-9276-static-templates January 18, 2021 10:50
Base automatically changed from DHIS2-9276-static-templates to DHIS2-9276-4-filters-only January 18, 2021 10:51
Base automatically changed from DHIS2-9276-4-filters-only to DHIS2-9276-3-datasource January 18, 2021 10:51
Base automatically changed from DHIS2-9276-3-datasource to DHIS2-9276-2-1-api-utils-epics January 18, 2021 10:52
Base automatically changed from DHIS2-9276-2-1-api-utils-epics to DHIS2-9276-2-columns January 18, 2021 11:40
Base automatically changed from DHIS2-9276-2-columns to DHIS2-9276-1-init January 18, 2021 11:41
Base automatically changed from DHIS2-9276-1-init to DHIS2-9276 January 18, 2021 11:44
Base automatically changed from DHIS2-9276 to master January 18, 2021 13:50
@JoakimSM

Copy link
Copy Markdown
Member Author

@paschalidi Sorry, I totally missed your comment here.

Thanks for the comment, this was really valuable feedback for me 👍 I lost the user perspective here somewhat.

Did some reading to get my head straight and a SO comment about functional testing hit the nail on the head for me:

Functional tests may be user facing, in which case they are written by developers together with users (or maybe with the right tools and right users even by the users themselves), for users, from the user's perspective. Or they may be developer facing (e.g. when they describe some internal piece of functionality that the user doesn't care about), in which case they are written by developers, for developers, but still from the user's perspective. In the former case, the functional tests may also serve as acceptance tests and as an executable encoding of functional requirements or a functional specification, in the latter case, they may also serve as integration tests.

So I believe many of my tests here by this definition is considered integration tests.

Anyway, I actually forgot about this PR when I merged the tei working lists earlier today, so I will close this one and create a new reworked PR.

Let's have a chat about this tomorrow if you have time.

@JoakimSM JoakimSM closed this Jan 18, 2021
@paschalidi

Copy link
Copy Markdown
Contributor

@JoakimSM yep would be up for a chat for sure :) Take it easy 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants