search

DickinsonCollege / FD2School-FarmData2-S23

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

Seeding Report: Test Cancel Seeding Log Edit #236

Open Shahir-47 opened 1 year ago

Shahir-47 commented 1 year ago

Pull Request Description

addresses #202

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.

Shahir-47 commented 1 year ago

@johnmaccormick @braughtg, you'll notice that in my cypress tests when I'm choosing a crop from the drop-down menu, I used an index value and not a specific crop name like "BEAN" to select an option from the crop dropdown menu.

cy.get('[data-cy=r0-Crop-input]').select(0)

This is because of the duplication issue I mentioned before at https://github.com/DickinsonCollege/FD2School-FarmData2/issues/202#issuecomment-1532411359 but now that I've fixed the duplication issue at #237. If you can get issue #237 merged, I can change the argument from an index to a specific crop name like the following:

cy.get('[data-cy=r0-Crop-input]').select("BEAN")

braughtg commented 1 year ago

If you can get issue #237 merged, I can change the...

@Shahir-47 My suggestion here would be to add the changes in this PR to PR #237 so that it contains both the bug fix and the corrected test that will pass with the bug fix. Then close this PR. That way, when #237 is merged both the test and the bug fix will go in together.

If you do this the body text for #237 should contain "Closes" lines for both issue tickets that are addressed. I notice that you've used "Addresses" instead of "Closes." Changing that to "Closes" will ensure that the issue tickets are closed automatically when the PR is mereged.

johnmaccormick commented 1 year ago

I'm getting timeout errors when I run your tests. Can you confirm that they all pass for you?

I am not currently using my usual machine. I will try later with another one to verify.

braughtg commented 1 year ago

I'm getting timeout errors when I run your tests. Can you confirm that they all pass for you?

image

They pass for me.

braughtg commented 1 year ago

I am going to change this back to draft pending the changes discussed above.

@Shahir-47 My suggestion here would be to add the changes in this PR to PR #237 so that it contains both the bug fix and the corrected test that will pass with the bug fix. Then close this PR. That way, when #237 is merged both the test and the bug fix will go in together.

I'll leave this PR and #237 open until the end of May if you have an interest in addressing this.

Shahir-47 commented 1 year ago

I'm getting timeout errors when I run your tests. Can you confirm that they all pass for you?

I am not currently using my usual machine. I will try later with another one to verify.

Hi @johnmaccormick, I think you're getting this error because the machine you're running tests on might not be rendering some of the columns as mentioned by @braughtg earlier. Hence, cypress can't get the elements that appear in the unrendered column. I mentioned this issue here: https://github.com/DickinsonCollege/FD2School-FarmData2/issues/202#issuecomment-1532404176 and there is also a issue tracker #230 on this error.

If done on the right machine, all of the tests do pass. Alternatively, changing the if condition if (window.screen.width < 900 || window.screen.height < 900) { this.isMobile = true } to this if (window.screen.width < 500 || window.screen.height < 500) { this.isMobile = true } in seedingReport.html did work and gave me a temporary way of fixing and checking my cypress tests.

There's nothing I can really do here unless issue #230 is fixed