-
Notifications
You must be signed in to change notification settings - Fork 355
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Regression tests fail on macOS due to Windows-based assumptions #2735
Comments
@mcking65, should this be even 2 issues? One the issue of the tests failing b/c of Windows-based assumptions, but the other of the remaining 2 tests that fail on macOS for ... 🤷🏽♀️. |
The ARIA Authoring Practices (APG) Task Force just discussed The full IRC log of that discussion<jugglinmike> Topic: Running tests on macOS<jugglinmike> Andrea_Cardona: Running this on my end, I can see a number of files checked and a timeout reported by Ava. It tells me that 57 hooks failed <jugglinmike> Matt_King: There are exactly 57 examples, so that's all of them <jugglinmike> Andrea_Cardona: I believe the errors I'm observing are different than what the reporter has shared <jugglinmike> github: https://github.com//issues/2735 <jugglinmike> Matt_King: Paul was following the instructions for new contributors and ran into this problem right here. It's quite distracting, and I'm very concerned that we have instructions for new contributors that are leading them astray <jugglinmike> Matt_King: We designed the regression tests to be able to run them locally, and I think they SHOULD be run locally, but if this is going to take a long time to address, I wonder if we should change the recommendation to rely on the CI system to run the regression tests <jugglinmike> Andrea_Cardona: Yeah, arigilmore and I have relied on the CI system in that way in the past <jugglinmike> Andrea_Cardona: It's somewhat slow, though! <jugglinmike> jugglinmike: It can also be intimidating for new contributors to make their first pull request. It would be intimidating to have to wait for this automated feedback until *after* you've shared your work publicly <jugglinmike> howard-e: They run reliably for me when I use Node.js 16 instead of Node.js 18. I'd be reluctant to pin to an old release of Node.js, though <jugglinmike> Zakim, end the meeting |
@frozenzia Before I can move on with this task, I just want to align on your Mac environment for running the regression tests. In
If you have any workarounds to this, please let me know. I also wanted to align that we are seeing the same regression results. My regression output without the proposed changes are as follows - not sure if they have changed:
I can confirm the 6 known failures. As for the other results, I'd like to make sure that we match. From my understanding, there were 5 failing tests upon the last comment in June, but now, there are 9 failing tests. |
@evmiguel Sorry for the delayed response - I'll try to get to this in the next day or two and get back to you. |
@evmiguel, alright, sorry, finally got to this. Using main branch, which had as its last commit 81d807d (from Feb 26, 2024). I have not had to touch In June the failing cases were in:
Now the failing cases are in:
AND, again if I make the same change to
With that change there are only 2 tests failed, 6 known failures. The 2 failed tests are in:
|
@frozenzia Thanks for getting back to me. I like your idea of detecting OS for managing edit fields. Are there any other tests you found that require this other than |
@frozenzia wrote:
In
toolbar_toolbar.js
, there are a couple of tests where CTRL+A are currently used to select all the text in a textarea. This does nothing on a Mac, and copying similar code fromalertdialog_alertdialog.js
, I've added an extra line to do the selecting in case the test is run on a Mac. After making this one and only change, running the tests still results in 2 test failures (+ 6 known failures):If I DON'T add those 2 lines in the toolbar test, I actually end up with 5 tests failed:
Originally posted by @frozenzia in #2628 (comment)
The text was updated successfully, but these errors were encountered: