DickinsonCollege / FD2School-FarmData2-S23

A fork of FarmData2 that is used for the FarmData2 School Activities.
Other
1 stars 36 forks source link

Presence of Whitespaces in Crop Name Retrieved From Database #215

Closed pranavm2109 closed 1 year ago

pranavm2109 commented 1 year ago

Description When the file farmdata2/farmdata2_modules/fd2_barn_kit/seedingReport/seedingReport.crop.filter.spec.js in PR number 196 is run in Cypress, using have.text in line 73 instead of contain.text in the third individual test causes an AssertionError, due to the presence of whitespaces in the crop name retrieved from the database, which causes it to not match with the expected crop name. Here, have.text needs to be used since the crop names need to match exactly with their expected value. The line contain.text checks to see if one string is a substring of another, which thereby does not achieve what is needed.

Step by Step Directions to Reproduce Bug

  1. Run the file seedingReport.crop.filter.spec.js found in PR number 196, in Cypress. It will pass, since at the moment, contain.text is used in the third individual test in the file in line 73, instead of have.text
  2. Replace contain.text with have.text in the third individual test in line 73, which checks to see that the report table only contains the logs pertaining to the selected crop.
  3. Save, and run the file seedingReport.crop.filter.spec.js in Cypress again.

Expected Result The expected result is that the whitespaces should not exist in the crop name retrieved from the database, causing the assert statement with have.text to pass. In the given example, this means that 'CAULIFLOWER' should be retrieved from the database, and not 'CAULIFLOWER '.

Observed Result

Screenshot 2023-04-13 at 2 32 12 PM

As it can be observed here, there is an AssertionError since the expected text 'CAULIFLOWER' does not match with the retrieved text 'CAULIFLOWER '.

braughtg commented 1 year ago

@pranavm2109 Good catch! This issue could have creeped in at a few points. Places to consider looking would be:

braughtg commented 1 year ago

Related to PR #196. The code changes in that PR should be revisited when this issue is resolved.

pranavm2109 commented 1 year ago

I would like to work on this please!

andrewscheiner53 commented 1 year ago

I would like to work on this please!

Alexandrialexie commented 1 year ago

I would like to work on this, please!

Alexandrialexie commented 1 year ago

Update: To be specific, there are exactly five white spaces added to each entry in the table, this included the entries in the other columns in the table as well. These spaces are also found in the transplanting report file. We believe that these spaces are coming from the <!-- --> lines that appear when you print the element with the data-cy=td-r0c1 tag to the console.

To see the <!-- -->: First, td-r0c1 tag, in the seedingReport,crop.filter.spec.js file replace the '[data-cy=td-r'+0+"c1]" on line 73 with '[data-cy=td-r'+0+"c1]". Then run the Cyprus tests in the seedingReport,crop.filter.spec.js file. Next, open the developer tools. Then, find the line in the tests marked get [data-cy=td-r0c1] (it should be right above the first failed test) and click on it to print to the console. The console should now look like this: image

It seems like the <!-- --> shown here are likely causing the extra spaces since there are five of them, so finding a way to remove them should fix the problem.

Work around for tests: Until this problem can be fixed, tests that wish to check the entirety of the contents of a cell in the table can use the data-cy value in the div (in the case of the crop column this is data-cy=r'+i+"-Crop . Thus, the test would look something like: cy.get('[data-cy=r'+i+"-Crop]").should('have.text', 'CAULIFLOWER').

braughtg commented 1 year ago

Update: To be specific, there are exactly five white spaces added to each entry in the table, this included the entries in the other columns in the table as well. These spaces are also found in the transplanting report file. We believe that these spaces are coming from the <!-- --> lines that appear when you print the element with the data-cy=td-r0c1 tag to the console.

Awesome! Thanks for the additional information on this issue.

Using the data-cy=ri-Crop attribute to select the cell being tested is the preferred solution so that is a great fix for your tests.

That said, the data-cy=tc-ricj attribute should also work - so there is an issue that needs to be addressed here. So investigating the code within the CustomTableComponent to understand where the ` come from and whether it would be possible to remove them would be a great help.

@Alexandrialexie @pranavm2109, I hope you will consider working on this issue once your tests are complete. If so you might post this issue to the upstream FarmData2 Issue Tracker (https://github.com/DickinsonCollege/FarmData2/issues) and then work on it in the live project.

pranavm2109 commented 1 year ago

@braughtg Yes, I would consider working on this issue. @Alexandrialexie what do you say?

Alexandrialexie commented 1 year ago

Further Update:

The problem appears to be in the td in the CustomTableComponent.js file with :data-cy="'td-r'+ri+'c'+ci". This element contains a div element (:data-cy="'r'+ri+'-'+columns[ci].header") and five additional elements (a textarea, select, two inputs, and a regex input). It is these five additional elements that seem to be causing the problem as deleting them removes the unwanted extra spaces. Of course, removing these elements is not a fix since they have their own purposes.

It seems like either farmOS or Cyprus testing must be converting these elements into the <!-- -->, and then those are being interpreted as blank spaces. The best solution seems to be to use data-cy=ri-Crop (or the equivalent attribute for the column being tested) whenever possible and to use contains.text when :data-cy="'td-ricj" must be used.