Fixing Presence of Whitespaces in Crop Name

[andrewscheiner53]

Pull Request Description

This pull request will investigate the presence of whitespace in crop names when retrieved from the database. The goal is to find the root of the problem - where and why whitespace appears in crop names - and hopefully find a solution to removing the whitespace. This would allow the farmdata2/farmdata2_modules/fd2_barn_kit/seedingReport/seedingReport.crop.filter.spec.js Cypress test file to use have.text instead of contains.text when checking for valid crop names.

Closes #196 Closes #215

---

Licensing Certification

FarmData2 is a Free Cultural Work and all accepted contributions are licensed as described in the LICENSE.md file. This requires that the contributor holds the rights to do so. By submitting this pull request I certify that I satisfy the terms of the Developer Certificate of Origin for its contents.

[braughtg]

Please change the "Addresses" lines in your PR description to "Closes" or "Resolves" or "Fixes" so that the PR will link correctly to the issue.

[andrewscheiner53]

Hi all, I made some discoveries on this issue recently and I wanted to make note of them. First, I investigated the td-ricj data-cy attribute which specifies the exact value for a given report table element. In our case, we are interested in any td-ric1 attribute since this will give us the crop name for the cell at index i. Using Cypress, I printed the element with the td-r0c1 attribute (specifically) to the Web Developer Tools Console. Here was what I found:

Observed Output

As you can see by the picture, the text within the <div> element has no whitespace. Yet, our Cypress test adds extra whitespace to the expected result shown here:

Observed Test Result

@braughtg I am trying to hypothesize reasons for why this is happening and here are my thoughts. I think it could be 1) the td-ricj data-cy attribute, 2) Vue's CustomTableComponent itself (or even the report-table in seedingReport.html), or 3) something to do with Cypress's 'have.text' method. Do you happen to have any ideas or thoughts on this issue? Thank you!

[andrewscheiner53]

In addition to investigating the td-ricj attribute, I also looked into new ways we could validate that CAULIFLOWER is the crop name that appears in our table when selected (exactly as is without worrying about whitespace). Before passing a commit, I wanted to post the code and let it be up for review. So, instead of using 'have.text' or 'contains.text' as a parameter to the should() method, here is a possible solution:

Starting at line 74 in seedingReport.crop.filter.spec.js:

cy.get('[data-cy=td-r'+i+"c1]")
    .then($crop => $crop.text().trim())
    .should('eq', 'CAULIFLOWER')

I did attempt this test before posting and here was the output:

Observed Test Output

The full test passed with no errors.

While trim() partially defeats our purpose, this method would allow us to remove whitespace no matter if the text actually contains whitespace or not. This also helps so that we don't have to check for substrings. We can then use the 'eq' argument in should() to more precisely test the crop name. @Alexandrialexie and @pranavm2109 Please let me know your thoughts on my proposed solution. @braughtg Would this be an acceptable fix whether or not we are able to find a solution to our issue?

[braughtg]

@braughtg I am trying to hypothesize reasons for why this is happening and here are my thoughts. I think it could be 1) the td-ricj data-cy attribute,

One possible thing to try here is to use the data-cy value of ri-Crop instead of the td-ricj value. See the documentation for the CustomTableComponent. Every column in a row has a ri-Name value where Name is based on the column header. This may help with your issue, and even if it does not it will make your test more readable since the reader will not need to just know what is in cj.

[braughtg]

Observed Output

As you can see by the picture, the text within the <div> element has no whitespace.

I wonder if this is related to the 5 HTML comments <!-- --> that are appearing in this output? When you get the element for td-ricj you are getting those along with the div containing the actual value. This makes me think even more strongly that using the ri-Crop value might solve this problem.

Of course it would still leave the question of where those <!----> are coming from and if that can be addressed so that other use cases where td-ricj is the right approach will work as expected.

[andrewscheiner53]

One possible thing to try here is to use the data-cy value of ri-Crop instead of the td-ricj value.

@braughtg That seems to have worked! Here is the code I used and the test output:

cy.get('[data-cy=r'+i+"-Crop]")
    .should('have.text', 'CAULIFLOWER')

Test Output

[braughtg]

While trim() partially defeats our purpose, this method would allow us to remove whitespace no matter if the text actually contains whitespace or not. This also helps so that we don't have to check for substrings. We can then use the 'eq' argument in should() to more precisely test the crop name. @Alexandrialexie and @pranavm2109 Please let me know your thoughts on my proposed solution. @braughtg Would this be an acceptable fix whether or not we are able to find a solution to our issue?

I would suggest trying the ri-Crop value approach described in my other two comments before resorting to using trim(). Which while it works, seems like a hack that is pointing out a larger issue with the code that should be addressed instead.

[pranavm2109]

I think we can use it with ri-Crop but the whitespaces should fundamentally not be present there irrespective of which data-cy attribute we choose. In the interest of time and progress, I suggest we proceed with this. @braughtg @andrewscheiner53 @Alexandrialexie what do you think?

[andrewscheiner53]

@pranavm2109 Sounds good to me. And yes I agree that we shouldn't have to worry about which data-cy attribute we choose and that there is likely a larger issue. I'll go ahead and pass the commit with the updated test.

[andrewscheiner53]

I wonder if this is related to the 5 HTML comments <!-- --> that are appearing in this output? When you get the element for td-ricj you are getting those along with the div containing the actual value.

@braughtg This is actually a really interesting point. Looking back at the original whitespace issue in the tracker, I remember that we found there to be 5 extra whitespaces in the expected value Cypress was testing for. Maybe each <!-- --> is adding a blank space to CAULIFLOWER in our original test.

@pranavm2109 @Alexandrialexie

[braughtg]

Looking back at the original whitespace issue in the tracker, I remember that we found there to be 5 extra whitespaces in the expected value Cypress was testing for.

Please add this new information to that issue so that someone attempting to work on that issue later will have this as a head start.

[braughtg]

@andrewscheiner53 @pranavm2109 @Alexandrialexie What is the status of this PR at this point? With the other related PR's merged, should this one now be closed? Or are the tests here something that should be reviewed and merged?

[andrewscheiner53]

@andrewscheiner53 @pranavm2109 @Alexandrialexie What is the status of this PR at this point? With the other related PR's merged, should this one now be closed? Or are the tests here something that should be reviewed and merged?

@braughtg I believe this branch can be closed. We fixed the method for our test in #196, so the changes in this PR are unnecessary. I think I accidentally closed this PR, is that ok?