Skip to content

test: Revert rudimentary console filtering - #323

Merged
fluiddot merged 1 commit into
trunkfrom
test/revert-rudimentary-console-filtering
Jul 1, 2024
Merged

test: Revert rudimentary console filtering#323
fluiddot merged 1 commit into
trunkfrom
test/revert-rudimentary-console-filtering

Conversation

@dcalhoun

Copy link
Copy Markdown
Member

Relates to #316.

Proposed Changes

Revert rudimentary console filtering
While this successfully reduced noise, it introduced indirection when
valid console errors log in tests, making it far more difficult to
decipher where an error originates. This trade-off felt like a
net-negative.

Example indirection in error log, where the stack trace points to the
filter, rather than the error origin itself:

Example error indirection
  console.error
    Warning: Each child in a list should have a unique "key" prop. See https://reactjs.org/link/warning-keys for more information.
        at /Users/davidcalhoun/Sites/a8c/studio/src/components/content-tab-assistant.tsx:76:3
        at div
        at div
        at div
        at ContentTabAssistant (/Users/davidcalhoun/Sites/a8c/studio/src/components/content-tab-assistant.tsx:172:40)

      55 |              }
      56 |
    > 57 |              level( ...args );
         |              ^
      58 |      };
      59 | }
      60 | console.log = filteredConsole( console.log );

      at console.error (jest-setup.ts:57:3)
      at printWarning (node_modules/react/cjs/react-jsx-runtime.development.js:87:30)
      at error (node_modules/react/cjs/react-jsx-runtime.development.js:61:7)
      at validateExplicitKey (node_modules/react/cjs/react-jsx-runtime.development.js:1078:5)
      at validateChildKeys (node_modules/react/cjs/react-jsx-runtime.development.js:1105:11)
      at jsxWithValidation (node_modules/react/cjs/react-jsx-runtime.development.js:1266:15)
      at jsxWithValidationStatic (node_modules/react/cjs/react-jsx-runtime.development.js:1296:12)
      at src/components/content-tab-assistant.tsx:105:4
      at renderWithHooks (node_modules/react-dom/cjs/react-dom.development.js:16305:18)
      at updateFunctionComponent (node_modules/react-dom/cjs/react-dom.development.js:19588:20)
      at updateSimpleMemoComponent (node_modules/react-dom/cjs/react-dom.development.js:19425:10)
      at beginWork (node_modules/react-dom/cjs/react-dom.development.js:21678:16)
      at beginWork$1 (node_modules/react-dom/cjs/react-dom.development.js:27426:14)
      at performUnitOfWork (node_modules/react-dom/cjs/react-dom.development.js:26560:12)
      at workLoopSync (node_modules/react-dom/cjs/react-dom.development.js:26466:5)
      at renderRootSync (node_modules/react-dom/cjs/react-dom.development.js:26434:7)
      at performConcurrentWorkOnRoot (node_modules/react-dom/cjs/react-dom.development.js:25738:74)
      at flushActQueue (node_modules/react/cjs/react.development.js:2667:24)
      at act (node_modules/react/cjs/react.development.js:2582:11)
      at node_modules/@testing-library/react/dist/act-compat.js:46:25
      at renderRoot (node_modules/@testing-library/react/dist/pure.js:161:26)
      at render (node_modules/@testing-library/react/dist/pure.js:247:10)
      at src/components/tests/content-tab-assistant.test.tsx:107:9
      at src/components/tests/content-tab-assistant.test.tsx:8:71
      at Object.<anonymous>.__awaiter (src/components/tests/content-tab-assistant.test.tsx:4:12)
      at Object.<anonymous> (src/components/tests/content-tab-assistant.test.tsx:104:73)

Testing Instructions

N/A, no user-facing changes.

Pre-merge Checklist

  • Have you checked for TypeScript, React or other console errors?
While this successfully reduced noise, it introduced indirection when
valid console errors log in tests, making it far more difficult to
decipher where an error originates. This trade-off felt like a
net-negative.

Example indirection in error log, where the stack trace points to the
filter, rather than the error origin itself:

```
  console.error
    Warning: Each child in a list should have a unique "key" prop. See https://reactjs.org/link/warning-keys for more information.
        at /Users/davidcalhoun/Sites/a8c/studio/src/components/content-tab-assistant.tsx:76:3
        at div
        at div
        at div
        at ContentTabAssistant (/Users/davidcalhoun/Sites/a8c/studio/src/components/content-tab-assistant.tsx:172:40)

      55 |              }
      56 |
    > 57 |              level( ...args );
         |              ^
      58 |      };
      59 | }
      60 | console.log = filteredConsole( console.log );

      at console.error (jest-setup.ts:57:3)
      at printWarning (node_modules/react/cjs/react-jsx-runtime.development.js:87:30)
      at error (node_modules/react/cjs/react-jsx-runtime.development.js:61:7)
      at validateExplicitKey (node_modules/react/cjs/react-jsx-runtime.development.js:1078:5)
      at validateChildKeys (node_modules/react/cjs/react-jsx-runtime.development.js:1105:11)
      at jsxWithValidation (node_modules/react/cjs/react-jsx-runtime.development.js:1266:15)
      at jsxWithValidationStatic (node_modules/react/cjs/react-jsx-runtime.development.js:1296:12)
      at src/components/content-tab-assistant.tsx:105:4
      at renderWithHooks (node_modules/react-dom/cjs/react-dom.development.js:16305:18)
      at updateFunctionComponent (node_modules/react-dom/cjs/react-dom.development.js:19588:20)
      at updateSimpleMemoComponent (node_modules/react-dom/cjs/react-dom.development.js:19425:10)
      at beginWork (node_modules/react-dom/cjs/react-dom.development.js:21678:16)
      at beginWork$1 (node_modules/react-dom/cjs/react-dom.development.js:27426:14)
      at performUnitOfWork (node_modules/react-dom/cjs/react-dom.development.js:26560:12)
      at workLoopSync (node_modules/react-dom/cjs/react-dom.development.js:26466:5)
      at renderRootSync (node_modules/react-dom/cjs/react-dom.development.js:26434:7)
      at performConcurrentWorkOnRoot (node_modules/react-dom/cjs/react-dom.development.js:25738:74)
      at flushActQueue (node_modules/react/cjs/react.development.js:2667:24)
      at act (node_modules/react/cjs/react.development.js:2582:11)
      at node_modules/@testing-library/react/dist/act-compat.js:46:25
      at renderRoot (node_modules/@testing-library/react/dist/pure.js:161:26)
      at render (node_modules/@testing-library/react/dist/pure.js:247:10)
      at src/components/tests/content-tab-assistant.test.tsx:107:9
      at src/components/tests/content-tab-assistant.test.tsx:8:71
      at Object.<anonymous>.__awaiter (src/components/tests/content-tab-assistant.test.tsx:4:12)
      at Object.<anonymous> (src/components/tests/content-tab-assistant.test.tsx:104:73)
```
@dcalhoun dcalhoun self-assigned this Jun 28, 2024
@dcalhoun
dcalhoun marked this pull request as ready for review June 28, 2024 12:03
@dcalhoun
dcalhoun requested a review from a team June 28, 2024 12:03
@fluiddot fluiddot mentioned this pull request Jul 1, 2024
1 task

@fluiddot fluiddot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 🎊 !

I'd propose using the jest-console package to explicitly check for console logs, as well as to identify potential logs we introduce by mistake.

@fluiddot
fluiddot merged commit e617a9a into trunk Jul 1, 2024
@fluiddot
fluiddot deleted the test/revert-rudimentary-console-filtering branch July 1, 2024 09:40
@dcalhoun

dcalhoun commented Jul 1, 2024

Copy link
Copy Markdown
Member Author

I'd propose using the jest-console package to explicitly check for console logs, as well as to identify potential logs we introduce by mistake.

Agreed, the project would greatly benefit from jest-console. IMO the project's current (seemingly intentional) logging quantity is a bit overbearing. As mentioned in the now-removed comment, I think a central logging utility should be introduced to improve logging consistency (formatting, included metadata, etc) and enable toggling the log output via a VERBOSE=true environment variable.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

2 participants