Skip to content

Conversation

@memwey
Copy link
Contributor

@memwey memwey commented Aug 15, 2025

问题在 #132 中修复, 我这里只保留了一些单元测试, 用以测试 grabNode, grabAllNode 等关键方法的行为

TODO: 更好的处理段落, 需要翻译的内容, 并将翻译好的内容添加回样式/超链接等行为

Summary by Sourcery

Set up Vitest-based testing environment, add unit tests for DOM node grabbing logic, and fix paragraph filtering in grabAllNode

Bug Fixes:

  • Fix grabAllNode to correctly include paragraphs containing only inline elements

Enhancements:

  • Refine node type checks and text length logic to account for inline elements
  • Add isContentEditableNode helper for cross-environment contentEditable detection

Build:

  • Add Vitest test scripts and dependencies in package.json
  • Introduce vitest.config.ts for configuring the test environment

Tests:

  • Implement Vitest setup file with jsdom and mocks for storage and browser APIs
  • Add unit tests for grabNode and grabAllNode functions

Chores:

  • Import storage in config.ts

@sourcery-ai
Copy link

sourcery-ai bot commented Aug 15, 2025

Reviewer's Guide

This PR integrates a Vitest-based testing framework with configuration and mocks, adds end-to-end unit tests for core DOM node grabbing utilities, and adjusts configuration imports to support stable test isolation.

Class diagram for updated config import and test setup

classDiagram
  class Config {
    +defaultValues
    +constructor()
  }
  class config {
    +Config instance
  }
  class storage {
    <<imported>>
  }
  config --> Config
  config ..> storage : uses

  class dom_test {
    <<test file>>
    +unit tests for grabNode
    +unit tests for grabAllNode
  }
  dom_test ..> config : imports
  dom_test ..> storage : uses (via config)

  class setup {
    <<test setup>>
    +mocks for storage
    +mocks for browser APIs
  }
  dom_test ..> setup : uses
Loading

File-Level Changes

Change Details Files
Integrate Vitest testing framework
  • Add test scripts (test, test:ui, test:run, test:coverage) to package.json
  • Introduce devDependencies: vitest, jsdom, happy-dom, Testing Library packages
  • Create vitest.config.ts to configure jsdom environment, globals, setupFiles, and aliases
package.json
vitest.config.ts
Set up global mocks and polyfills for test environment
  • Implement storage mocks for localStorage and sessionStorage with vi.fn hooks
  • Mock browser.runtime.sendMessage and webextension-polyfill APIs
  • Polyfill Element.prototype.getAttributeNames for JSDOM environments
test/setup.ts
Add unit tests for DOM node grabbing utilities
  • Create dom.test.ts covering grabNode, grabAllNode and related behaviors
  • Validate inline vs block element skipping, contentEditable detection, and text size checks
test/entrypoints/main/dom.test.ts
Adjust configuration import for storage
  • Import storage helper in config.ts to enable storage-related functionality in tests
entrypoints/utils/config.ts

Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

Copy link

@sourcery-ai sourcery-ai bot left a comment

Choose a reason for hiding this comment

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

Hey there - I've reviewed your changes and they look great!

Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments

### Comment 1
<location> `entrypoints/main/dom.ts:177` </location>
<code_context>
 }

+// 检查节点是否可编辑(兼容 JSDOM 环境)
+function isContentEditableNode(node: any): boolean {
+    return node.isContentEditable || 
+           node.contentEditable === 'true' ||
</code_context>

<issue_to_address>
Consider normalizing contentEditable attribute checks.

Normalize the contentEditable value to lower case before comparison to ensure case-insensitive checks.
</issue_to_address>

<suggested_fix>
<<<<<<< SEARCH
function isContentEditableNode(node: any): boolean {
    return node.isContentEditable || 
           node.contentEditable === 'true' ||
           node.getAttribute?.('contenteditable') === 'true';
}
=======
function isContentEditableNode(node: any): boolean {
    return node.isContentEditable ||
        (typeof node.contentEditable === 'string' && node.contentEditable.toLowerCase() === 'true') ||
        (typeof node.getAttribute === 'function' && typeof node.getAttribute('contenteditable') === 'string' && node.getAttribute('contenteditable').toLowerCase() === 'true');
}
>>>>>>> REPLACE

</suggested_fix>

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

Comment on lines 177 to 181
function isContentEditableNode(node: any): boolean {
return node.isContentEditable ||
node.contentEditable === 'true' ||
node.getAttribute?.('contenteditable') === 'true';
}
Copy link

Choose a reason for hiding this comment

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

suggestion: Consider normalizing contentEditable attribute checks.

Normalize the contentEditable value to lower case before comparison to ensure case-insensitive checks.

Suggested change
function isContentEditableNode(node: any): boolean {
return node.isContentEditable ||
node.contentEditable === 'true' ||
node.getAttribute?.('contenteditable') === 'true';
}
function isContentEditableNode(node: any): boolean {
return node.isContentEditable ||
(typeof node.contentEditable === 'string' && node.contentEditable.toLowerCase() === 'true') ||
(typeof node.getAttribute === 'function' && typeof node.getAttribute('contenteditable') === 'string' && node.getAttribute('contenteditable').toLowerCase() === 'true');
}

if (typeof Element !== 'undefined' && Element.prototype && typeof Element.prototype.getAttributeNames === 'undefined') {
console.log('Polyfilling Element.prototype.getAttributeNames');
Element.prototype.getAttributeNames = function getAttributeNames() {
const attributes = this.attributes;
Copy link

Choose a reason for hiding this comment

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

suggestion (code-quality): Prefer object destructuring when accessing and using properties. (use-object-destructuring)

Suggested change
const attributes = this.attributes;
const {attributes} = this;


ExplanationObject destructuring can often remove an unnecessary temporary reference, as well as making your code more succinct.

From the Airbnb Javascript Style Guide

@Bistutu
Copy link
Owner

Bistutu commented Aug 16, 2025

感谢pull request!这里可以提供1个范例吗

@memwey
Copy link
Contributor Author

memwey commented Aug 16, 2025

感谢pull request!这里可以提供1个范例吗

有的, 之前

这两个页面一些段落中带有超链接的话, 会被直接忽略掉

@memwey memwey changed the title fix(dom): fix paragraph skip when hyperlink included, add tests chore(tests): add unit tests Aug 18, 2025
@memwey
Copy link
Contributor Author

memwey commented Aug 18, 2025

@sourcery-ai review

Copy link

@sourcery-ai sourcery-ai bot left a comment

Choose a reason for hiding this comment

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

Hey there - I've reviewed your changes and they look great!


Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

if (typeof Element !== 'undefined' && Element.prototype && typeof Element.prototype.getAttributeNames === 'undefined') {
console.log('Polyfilling Element.prototype.getAttributeNames');
Element.prototype.getAttributeNames = function getAttributeNames() {
const attributes = this.attributes;
Copy link

Choose a reason for hiding this comment

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

suggestion (code-quality): Prefer object destructuring when accessing and using properties. (use-object-destructuring)

Suggested change
const attributes = this.attributes;
const {attributes} = this;


ExplanationObject destructuring can often remove an unnecessary temporary reference, as well as making your code more succinct.

From the Airbnb Javascript Style Guide

@memwey
Copy link
Contributor Author

memwey commented Aug 18, 2025

@sourcery-ai summary

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants