Closed beszel closed 4 years ago
@ProfJanetDavis On the failing e2e tests:
I'm not able to reproduce those errors, and my working tree is clean. Some of these test failures look like they're caused by the test race condition, so I suspect they all are.
Have you tried running the e2e tests on the master
branch and then immediately on the options
branch, several times each? If you do, make sure to leave Chromium in the foreground so we can control for that too. I suspect that the tests fail only when your computer is working hard on something else, so this experiment helps control for that.
If the problem is that the tests run before the extension makes changes, we should be able to fix most or all of them just by waiting in the before
block for a DGtW-added element to appear on the page. I'd start now but since I can't reproduce the test failures I wouldn't know if or when it worked.
Thanks- I wondered if it was a race condition. My laptop is pretty old. Let me try some of the things you suggest and see if I can resolve it.
Even if it might be helpful, I’m not available to meet at noon today - I’m getting a new nanny oriented and that’s the start of nap time. I’ll let you know the results if my experiments.
-- Janet Davis Sent from my "smart" phone, in haste, with auto-(in)correct
On May 26, 2020, at 8:21 PM, Ian Hawkins notifications@github.com wrote:
@ProfJanetDavis On the failing e2e tests: I'm not able to reproduce those errors, and my working tree is clean. Some of these test failures look like they're caused by the test race condition, so I suspect they all are. Have you tried running the e2e tests on the master branch and then immediately on the options branch, several times each? If you do, make sure to leave Chromium in the foreground so we can control for that too. I suspect that the tests fail only when your computer is working hard on something else, so this experiment helps control for that. If the problem is that the tests run before the extension makes changes, we should be able to fix most or all of them just by waiting in the before block for a DGtW-added element to appear on the page. I'd start now but since I can't reproduce the test failures I wouldn't know if or when it worked.
— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or unsubscribe.
@irhawk, you nailed it. If I keep Chrome in the foreground those e2e test errors go away. However, there is one error that's persistent:
1) When the extension is turned off for this host, the page
When the user has not yet reloaded the page, it should not yet change the status message: AssertionError: expected '' to equal 'Degender the Web has replaced
gender pronouns on this page.'
+ expected - actual +Degender the Web has replaced gender pronouns on this page. at Context.<anonymous>
(test/e2e/content/case.turned-off-for-host.test.js:43:35)
at processTicksAndRejections (internal/process/task_queues.js:85:5)
Is it possible this is a race condition also? Do you want to investigate, or give me a suggestion for where to start? Thanks!
On Wed, May 27, 2020 at 9:22 AM davisj@whitman.edu wrote:
Thanks- I wondered if it was a race condition. My laptop is pretty old. Let me try some of the things you suggest and see if I can resolve it.
Even if it might be helpful, I’m not available to meet at noon today - I’m getting a new nanny oriented and that’s the start of nap time. I’ll let you know the results if my experiments.
-- Janet Davis Sent from my "smart" phone, in haste, with auto-(in)correct
On May 26, 2020, at 8:21 PM, Ian Hawkins notifications@github.com wrote:
@ProfJanetDavis https://github.com/ProfJanetDavis On the failing e2e tests: I'm not able to reproduce those errors, and my working tree is clean. Some of these test failures look like they're caused by the test race condition, so I suspect they all are. Have you tried running the e2e tests on the master branch and then immediately on the options branch, several times each? If you do, make sure to leave Chromium in the foreground so we can control for that too. I suspect that the tests fail only when your computer is working hard on something else, so this experiment helps control for that. If the problem is that the tests run before the extension makes changes, we should be able to fix most or all of them just by waiting in the before block for a DGtW-added element to appear on the page. I'd start now but since I can't reproduce the test failures I wouldn't know if or when it worked.
— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/glam-lab/degender-the-web/pull/122#issuecomment-634404086, or unsubscribe https://github.com/notifications/unsubscribe-auth/AEXJIUZSPP4A2WFF5UM2DFDRTSBNLANCNFSM4M5VX6MA .
-- Janet Davis (she/her or they/them) Associate Professor and Microsoft Chair of Computer Science, Whitman College 509-527-5758, http://cs.whitman.edu/~davisj/ Schedule an appointment: http://meetme.so/JanetDavis
@ProfJanetDavis I'm glad it helped! That looks like a race condition where some test needs to wait for the popup status message, I'll take a look tonight. I expect to have a code snippet or temporary branch for you to try.
@ProfJanetDavis Check out the options-experiment
branch and try running the e2e tests there. I made the change we discussed before where the popup message remains hidden until filled, and tests can wait for it to appear. The test passes for me, but it passed before this change too (unfortunately).
If this works for you, make sure to run the tests again on options
right away to make sure a fluke didn't cause the tests to pass even without these changes.
As a next step, try uncommenting the page.waitFor
line I added in the before
block. It's not a real solution, but it will give us an idea of whether there's a race condition between the changes to Chrome storage and the extension applying to the page.
Good advice. The error in the options branch does indeed seem to be non-deterministic - those tests sometimes all passed. However,
I'm not sure which line of which file you are suggesting I uncomment, or in which branch. I tried a few things and all resulted in multiple test failures. Can you clarify? Or is that experiment moot with the success of the one above?
On Wed, May 27, 2020 at 10:07 PM Ian Hawkins notifications@github.com wrote:
@ProfJanetDavis https://github.com/ProfJanetDavis Check out the options-experiment branch and try running the e2e tests there. I made the change we discussed before where the popup message remains hidden until filled, and tests can wait for it to appear. The test passes for me, but it passed before this change too (unfortunately).
If this works for you, make sure to run the tests again on options right away to make sure a fluke didn't cause the tests to pass even without these changes.
As a next step, try uncommenting the page.waitFor line I added in the before block. It's not a real solution, but it will give us an idea of whether there's a race condition between the changes to Chrome storage and the extension applying to the page.
— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/glam-lab/degender-the-web/pull/122#issuecomment-635103896, or unsubscribe https://github.com/notifications/unsubscribe-auth/AEXJIU6NN2DQO3AI5GZ6JR3RTXWRHANCNFSM4M5VX6MA .
-- Janet Davis (she/her or they/them) Associate Professor and Microsoft Chair of Computer Science, Whitman College 509-527-5758, http://cs.whitman.edu/~davisj/ Schedule an appointment: http://meetme.so/JanetDavis
Perfect! If that works consistently, we don't need to try the second approach. I've cleaned up the experimental stuff and removed the commented-out "plan B" lines, then I merged it into the options
branch. I also clarified the comment you pointed out earlier, let me know what you think.
Merged to master and made a small formatting improvement to the popup. I think we are all set. Thanks for all your work on this!
Let me know if you want me to write a LinkedIn testimonial - or if you want to keep working on this as a personal project, for that matter.
Janet
On Thu, May 28, 2020 at 2:29 PM Ian Hawkins notifications@github.com wrote:
Perfect! If that works consistently, we don't need to try the second approach. I've cleaned up the experimental stuff and removed the commented-out "plan B" lines, then I merged it into the options branch. I also clarified the comment you pointed out earlier, let me know what you think.
— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/glam-lab/degender-the-web/pull/122#issuecomment-635618531, or unsubscribe https://github.com/notifications/unsubscribe-auth/AEXJIU6JXFT3QQSSKQJOJFTRT3JU5ANCNFSM4M5VX6MA .
-- Janet Davis (she/her or they/them) Associate Professor and Microsoft Chair of Computer Science, Whitman College 509-527-5758, http://cs.whitman.edu/~davisj/ Schedule an appointment: http://meetme.so/JanetDavis
Allows the user to turn off the extension for a host using either the popup or the new options page. This is an early PR and is still missing end-to-end tests for the popup interface, which will be added before this is resolved. These features are also minimally styled, and the style will not be finished as part of this PR.
To test these changes, experiment with the popup and the options page. The options page can be accessed from the DGtW extension page: chrome://extensions/?options=kgeehecadkggegiegoamiabpdjpgjkhg I can add more detailed manual testing instructions if appropriate.
Resolves #5, resolves #118.