PF4: (virtualized-extension) jest tests for VirtualizedTable by jenny-s51 · Pull Request #2761 · patternfly/patternfly-react · GitHub

WIP for #2328 -- currently working on resolving these type errors shown after running yarn run jest virtualizedTable.

Resolving TS errors with any type for now. Can change the types to what's more appropriate, but need some feedback on what would be best here. In addition, /Virtualized and/Virtualized/utils are now showing up in the coverage report, but coverage still really needs to be improved (13.23% and 19.29%, respectively). Will be working on this.

hey @jenny-s51 - my only ounce of feedback so far here would be - can we move this declaration into patternfly-4/react-virtualized-extension some place? I believe linear-layout-vector is only a dependency of that package (so probably unnecessary in react-core).

Also, i think you can safely remove package-lock.json as well. We don't ship lock files to consumers (just reference the dependences in the package.json for each package) and all of the dev dependencies for PF React end up getting locked in the root yarn.lock file (which should update when you install new packages or run yarn install in the root).

Great job w/ this, it's a huge help!!! ⭐️

hey @jenny-s51 - i've pushed a commit w/ some proposed changes to this PR today:
https://github.com/priley86/patternfly-react/commits/iss2328

Let me know what you think. I'm mostly happy here aside from one warning in the jest log still...

 PASS  packages/patternfly-4/react-virtualized-extension/src/components/Virtualized/VirtualizedTable.test.tsx
  ● Console

    console.error node_modules/prop-types/checkPropTypes.js:20
      Warning: Failed prop type: Invalid prop `children` supplied to `Table`, expected a ReactNode.
          in Table
    console.error packages/patternfly-4/react-table/dist/js/components/Table/Table.js:237
      Table: Specify at least one of: header, caption, aria-label
    console.error node_modules/prop-types/checkPropTypes.js:20
      Warning: Failed prop type: Invalid prop `children` supplied to `Provider`, expected a ReactNode.
          in Provider (created by Table)
          in Table (created by WrapperComponent)
          in WrapperComponent
    console.error node_modules/react-dom/cjs/react-dom.development.js:180
      Warning: Functions are not valid as a React child. This may happen if you return a Component instead of <Componen
t /> from render. Or maybe you meant to call this function rather than return it.
          in table (created by Provider)
          in Provider (created by Table)
          in Table (created by WrapperComponent)
          in WrapperComponent
    console.error packages/patternfly-4/react-table/dist/js/components/Table/Table.js:237
      Table: Specify at least one of: header, caption, aria-label

Did not get a chance to address it. I will try testing these changes downstream tomorrow and report back though!

PatternFly-React preview: https://patternfly-react-pr-2761.surge.sh

👍

i don't think this is needed in the root anymore since we've added it in react-virtualized-extension's package.json

how would you suggest I address this? would it just be yarn remove @types/react-virtualized?

I think so, yes, or you could remove the line from the package.json and re-run yarn install. If the build still passes without this dependency in here, yeah, I think remove it. But it's probably ok either way.

You can delete the line. Likely no need to re-run yarn install, but better safe than sorry.

👍

is this still necessary? (changing import to require)

hey @jenny-s51 - i'm mostly OK w/ this change. I would just remove any package-lock.json files (since we havent been checking those in)

@jenny-s51 +1 about package-lock.json.. and actually, if that file is being created you're probably using the npm CLI, I'd make sure you're running yarn install instead of npm install because we do want any updates to yarn.lock 🙂

@priley86 do you mean I can just remove package-lock.json from this PR entirely?

@jenny-s51 We use yarn with yarn.lock to install packages, so yes please remove package-lock.json!

@redallen gotcha, should be all set now -- thank you!

Codecov Report

Merging #2761 into master will decrease coverage by 2%.
The diff coverage is 36.36%.

@@            Coverage Diff             @@
##           master    #2761      +/-   ##
==========================================
- Coverage   69.13%   67.12%   -2.01%     
==========================================
  Files         868      873       +5     
  Lines       23777    24490     +713     
  Branches     1932     2071     +139     
==========================================
+ Hits        16438    16439       +1     
- Misses       6360     7088     +728     
+ Partials      979      963      -16

Flag	Coverage Δ
#misc	95.45% <ø> (ø)	⬆️
#patternfly3	69.3% <ø> (ø)	⬆️
#patternfly4	63.99% <36.36%> (-4.3%)	⬇️

Impacted Files	Coverage Δ
...ts/Virtualized/utils/CellSizeAndPositionManager.ts	16.49% <ø> (ø)
...src/components/Virtualized/utils/animationFrame.ts	27.27% <ø> (ø)
...onents/Virtualized/utils/createCallbackMemoizer.ts	0% <0%> (ø)
...components/Virtualized/defaultCellRangeRenderer.ts	1.81% <0%> (ø)
...on/src/components/Virtualized/VirtualTableBody.tsx	18.96% <0%> (ø)
...ualized/utils/ScalingCellSizeAndPositionManager.ts	27.94% <100%> (ø)
...nents/Virtualized/utils/requestAnimationTimeout.ts	27.27% <100%> (ø)
...xtension/src/components/Virtualized/VirtualGrid.ts	9.3% <33.33%> (ø)
...-4/react-core/src/components/TextArea/TextArea.tsx	93.75% <0%> (-1.71%)	⬇️
...ly-4/react-core/src/components/Tooltip/Tooltip.tsx	84.48% <0%> (-0.27%)	⬇️
... and 30 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 672047a...48756dd. Read the comment docs.

removed

…ors with any type for now

jenny-s51 requested a review from priley86 August 22, 2019 19:43

priley86 reviewed Aug 23, 2019

View reviewed changes

tlabaj assigned dlabaj and mturley and unassigned mturley Aug 29, 2019

tlabaj requested a review from mturley August 29, 2019 14:50

jenny-s51 force-pushed the iss2328 branch from ed86deb to c79f917 Compare September 9, 2019 16:38

jenny-s51 force-pushed the iss2328 branch from 824aee7 to 2e10b10 Compare September 16, 2019 16:21

jenny-s51 force-pushed the iss2328 branch from 3790ffd to 2e10b10 Compare September 16, 2019 16:48

priley86 reviewed Sep 25, 2019

View reviewed changes

priley86 requested review from redallen and seanforyou23 September 25, 2019 19:27

jenny-s51 force-pushed the iss2328 branch from 577a6e2 to c539616 Compare September 27, 2019 16:41

jenny-s51 requested a review from priley86 September 27, 2019 16:43

jenny-s51 force-pushed the iss2328 branch from b929d46 to 29ebfa6 Compare October 15, 2019 18:32

jenny-s51 force-pushed the iss2328 branch 3 times, most recently from 36a8485 to 5713a58 Compare October 15, 2019 22:45

jenny-s51 dismissed mturley’s stale review via c761c1d October 24, 2019 23:53

jenny-s51 force-pushed the iss2328 branch 4 times, most recently from be32d96 to 6921275 Compare October 25, 2019 00:49

jenny-s51 and others added 14 commits October 25, 2019 12:25

[WIP] jest tests for VirtualizedTable

87d0214

add(virtualized table tests): tests are now running, resolving TS err…

9bec6ea

…ors with any type for now

add yarn.lock

072e3da

Delete package-lock.json

6e8aa60

fix table tests and make rows prop optional

cffa2d6

resolving conflict in yarn.lock

61f85e9

addressing PR feedback

1d77079

removing react-virtualized dependency from root

dbaaf6c

resolving merge conflicts and trying to fix snapshots

37463d2

updates snapshots

1697b25

updates snapshot test

38cc6f5

Update VirtualizedTable.test.tsx

c8983ef

updates package.json and yarn.lock

c92880c

update package.json and rerun yarn install

3de0bfb

jenny-s51 force-pushed the iss2328 branch from 38301b6 to 3de0bfb Compare October 25, 2019 16:38

Delete package-lock.json

48756dd

jenny-s51 requested a review from mturley October 25, 2019 16:54

redallen approved these changes Oct 25, 2019

View reviewed changes

mturley approved these changes Oct 25, 2019

View reviewed changes

mturley merged commit bb31325 into patternfly:master Oct 25, 2019

jenny-s51 deleted the iss2328 branch October 25, 2019 17:21

tlabaj mentioned this pull request Oct 25, 2019

PF4: React Virtualized tests #2328

Closed

redallen added a commit to redallen/patternfly-react that referenced this pull request Oct 28, 2019

fix(virtualized): fix regressions from patternfly#2761

584ab2e

kmcfaul pushed a commit that referenced this pull request Oct 29, 2019

fix(virtualized): fix regressions from #2761 (#3228 )

73ec92e

                 dropdownPosition?: 'right' | 'left';
                 dropdownDirection?: 'up' | 'down';
-                rows: (IRow | string[])[];
+                rows?: (IRow | string[])[];

                 export const TextSize: any;
               }
+              declare module 'linear-layout-vector' {

                   ]
+                },
+                "dependencies": {
+                  "@types/react-virtualized": "^9.21.4"

                 expect(view).toMatchSnapshot();
               });
+              test('Header only table', () => {

               /* eslint-disable */
-              import LinearLayoutVector from 'linear-layout-vector';
+              const LinearLayoutVector = require("linear-layout-vector");

PF4: (virtualized-extension) jest tests for VirtualizedTable by jenny-s51 · Pull Request #2761 · patternfly/patternfly-react · GitHub

Uh oh!

PF4: (virtualized-extension) jest tests for VirtualizedTable - #2761

PF4: (virtualized-extension) jest tests for VirtualizedTable#2761mturley merged 15 commits intopatternfly:masterpatternfly/patternfly-react:masterfrom jenny-s51:iss2328jenny-s51/patternfly-react:iss2328Copy head branch name to clipboard

Conversation

jenny-s51 commented Aug 22, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jenny-s51 commented Aug 23, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

priley86 commented Sep 9, 2019

Uh oh!

patternfly-build commented Sep 16, 2019

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

priley86 commented Sep 25, 2019

Uh oh!

mturley commented Sep 25, 2019

Uh oh!

jenny-s51 commented Sep 26, 2019

Uh oh!

redallen commented Sep 26, 2019

Uh oh!

jenny-s51 commented Sep 26, 2019

Uh oh!

codecov-io commented Oct 15, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

PF4: (virtualized-extension) jest tests for VirtualizedTable#2761
mturley merged 15 commits into
patternfly:masterfrom
jenny-s51:iss2328

jenny-s51 commented Aug 22, 2019 •
edited
Loading

jenny-s51 commented Aug 23, 2019 •
edited
Loading

codecov-io commented Oct 15, 2019 •
edited
Loading