Fix failing tests in the main branch #941
Conversation
Codecov Report
@@ Coverage Diff @@
## main #941 +/- ##
=======================================
Coverage 99.29% 99.29%
=======================================
Files 104 104
Lines 1554 1554
Branches 522 522
=======================================
Hits 1543 1543
Misses 11 11
|
jessebeach
force-pushed
the
fix-main-branch-failing-tests
branch
from
ad78c86
to
3bac0f7
Compare
ljharb
force-pushed
the
fix-main-branch-failing-tests
branch
2 times, most recently
from
56b06de
to
5749b1b
Compare
jessebeach
force-pushed
the
fix-main-branch-failing-tests
branch
from
5749b1b
to
1d4eac1
Compare
|
Finally found some time (and focus) on a flight to get back to this. |
jessebeach
force-pushed
the
fix-main-branch-failing-tests
branch
from
1d4eac1
to
26e8e19
Compare
.npmrc
Outdated
| @@ -1,3 +1,4 @@ | |||
| package-lock=false | |||
| allow-same-version=true | |||
| message=v%s | |||
| audit-level=low | |||
There was a problem hiding this comment.
Just testing a theory.
There was a problem hiding this comment.
@ljharb I'm going to need help with this one. It seems we're in a bind. We can't update semver because of this package's commitment Node 4 support. Here are some options
- We could turn off the
posttestchecks for now, until the Node 4 is no longer supported in this project andsemvercan be updated. - Comment out the usages of
semverin the project -- which means our test coverage for optional chaining is reduced -- and remove the dependency. - Update
semverand drop Node 4 support. - Move the test cases that are wrapped in the
semvercheck to their own folder and configure ESLint to only run on them if the version of ESLint is greater than v6.
I'm going to try #4, but I'm on a plane right now and internet is spotty, so please ignore some thrash in the PR as I use it to test out some theories.
There was a problem hiding this comment.
Don't worry about the posttest tests due to the semver false positive CVE - if that's the only failure, i can take care of that. There's still a bunch of failing node tests tho.
There was a problem hiding this comment.
Actually, if you rebase on top of #944 instead of main, then that should address the problem :-) that way i can merge both PRs at once once the other tests are passing.
| @@ -340,6 +339,7 @@ const neverValid = [ | |||
| { code: '<style onClick={() => {}} />;', errors: [expectedError] }, | |||
| { code: '<sub onClick={() => {}} />;', errors: [expectedError] }, | |||
| { code: '<sup onClick={() => {}} />;', errors: [expectedError] }, | |||
| { code: '<summary onClick={() => {}} />;', errors: [expectedError] }, | |||
There was a problem hiding this comment.
I'm not sure if this test adjustment is right, this may be an issue in axobject-query: A11yance/axobject-query#319
There was a problem hiding this comment.
@dummdidumm ya, I think you're right. This is a tricky one. More details here: https://html.spec.whatwg.org/multipage/interactive-elements.html#the-summary-element
Given that we need to know the rendered output to make a determination about this element's role and behavior, the best course of action for a static linter is to be permissive of either state - an interactive element or a generic (static) element. I'll get this updated when I do a proper update of aria-query for ARIA 1.3.
There was a problem hiding this comment.
@dummdidumm #656 to tracks this.
|
@jessebeach - update on this? I'm waiting to update due to the semver being in I noticed it's in #944 also. Maybe we shouldn't combine the PRs and have #944 merged first? |
|
@karlnorling semver v5, v6, and v7 all have patches that fix that issue, so nobody should need to do anything except end users updating their lockfiles. |
ljharb
force-pushed
the
fix-main-branch-failing-tests
branch
2 times, most recently
from
4e7c7b9
to
b32d046
Compare
|
Looks like axe-core broke us too: dequelabs/axe-core#4127 |
ljharb
force-pushed
the
fix-main-branch-failing-tests
branch
from
b32d046
to
8d8f016
Compare
|
Last 2 failing tests seem to be about error messages for two tests showing up in the wrong order. I can't reproduce them locally, consistently. |
… kill the install process in npm < 6 See isaacs/jackspeak#4
…node This has the side benefit of making the error message ordering consistent, since when both were on the JSXOpeningElement, since eslint/node combinations ordered the errors inconsistently
@karlnorling, @ljharb merged it yesterday. |
There are a few failing tests in the main branch and this fixes them.
This PR also pegs
aria-queryto version 5.1.2.