-
Notifications
You must be signed in to change notification settings - Fork 0
{pr_title} #57
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
base: main
Are you sure you want to change the base?
{pr_title} #57
Conversation
…handling and support for BML operations in admin tools
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
…debugging, and remove deprecated files
…, and ensure graceful handling of missing DOM elements
…streamline message sending
…ton event handler setup
Bumps [tar-fs](https://github.com/mafintosh/tar-fs) to 3.0.8 and updates ancestor dependency [puppeteer](https://github.com/puppeteer/puppeteer). These dependencies need to be updated together. Updates `tar-fs` from 3.0.5 to 3.0.8 - [Commits](mafintosh/tar-fs@v3.0.5...v3.0.8) Updates `puppeteer` from 22.11.2 to 22.15.0 - [Release notes](https://github.com/puppeteer/puppeteer/releases) - [Changelog](https://github.com/puppeteer/puppeteer/blob/main/CHANGELOG.md) - [Commits](puppeteer/puppeteer@puppeteer-v22.11.2...puppeteer-v22.15.0) --- updated-dependencies: - dependency-name: tar-fs dependency-type: indirect - dependency-name: puppeteer dependency-type: direct:production ... Signed-off-by: dependabot[bot] <support@github.com>
…f3b7f925 Bump tar-fs and puppeteer
…njected module and update import path in tests
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. 🗂️ Base branches to auto review (2)
Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
…eScript and adjust wallaby version
…loading and unloading JSON/XML
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
…into typescript
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Introduces descriptive titles for various popup HTML files to improve document clarity and accessibility. Corrects a typo in the script reference for the SOAP popup interface. Enhances unit test environment setup with the addition of the Jest DOM environment.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR adds Wallaby.js support, extends Jest-based unit tests (including popup and admin scripts), refactors the main popup script for improved modularity and debug logging, reorganizes several popup HTML templates (renaming files and updating background references), and updates dependencies and configuration files.
- Configure Wallaby.js for Chrome extension tests
- Expand and deduplicate Jest tests for popup functionality and admin injections
- Refactor
src/popup/popup.js(URL analysis, event listener wiring,saveText,sanitizeFilename) - Rename popup HTML templates and update
background.jsto point to new files
Reviewed Changes
Copilot reviewed 30 out of 40 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/wallaby.js | Add Wallaby.js configuration with bootstrap/teardown for Puppeteer |
| tests/unit/popup/popup2.test.js | Add new suite of unit tests for popup V2 |
| tests/unit/popup/popup.test.js | Extend existing popup tests, mock handlers, simulate clicks |
| src/popup/popup.js | Major refactor: extract URL analysis, modularize handlers, replace onclicks with addEventListener |
| src/popup/*.html | Rename and update multiple popup HTML templates with aria-labels and new file names |
| src/background/background.js | Update popup file paths to .html variants with HTML suffix |
| package.json | Update devDependencies, bump Jest, add Wallaby and Babel |
Files not reviewed (1)
- .cpqdevkit/libs/package-lock.json: Language not supported
Comments suppressed due to low confidence (1)
tests/unit/popup/popup.test.js:13
- The tests use element IDs like
loadTestButtonandunloadButton, but the actual popup HTML defines IDsloadTest,unload, etc. Update the test IDs to match the real markup.
let mockSendMessage, mockExecuteScript, mockQuery;
| testFramework: 'jest', | ||
|
|
||
| setup: function(wallaby) { | ||
| const jestConfig = require('./package.json').jest; |
Copilot
AI
Jul 3, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The require path points to tests/package.json which likely doesn't exist. Update to require('../package.json').jest so it correctly loads the project’s Jest configuration.
| const jestConfig = require('./package.json').jest; | |
| const jestConfig = require('../package.json').jest; |
| logDebug("Options button clicked, redirecting to options page."); | ||
| window.location = '/options.html'; | ||
| } | ||
| logDebug("Extension initialized."); |
Copilot
AI
Jul 3, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Event listeners for the ‘options’ and ‘logs’ buttons were removed during refactoring. Reintroduce handlers (e.g. document.getElementById('options').addEventListener('click', …)) to restore navigation and logging functionality.
| /** | ||
| * Unit tests for popup.js | ||
| */ | ||
|
|
||
| describe('Popup v2 Script', () => { | ||
| let mockSendMessage, mockExecuteScript, mockQuery; | ||
|
|
||
| beforeEach(() => { | ||
| // Mock Chrome APIs | ||
| global.chrome = { | ||
| tabs: { | ||
| query: jest.fn(), | ||
| sendMessage: jest.fn(), | ||
| }, | ||
| scripting: { | ||
| executeScript: jest.fn(), | ||
| }, | ||
| runtime: { | ||
| getManifest: jest.fn(() => ({ | ||
| name: 'Streamline Tools', | ||
| version: '1.0.0', | ||
| })), | ||
| }, | ||
| downloads: { | ||
| setShelfEnabled: jest.fn(), | ||
| onDeterminingFilename: { | ||
| addListener: jest.fn(), | ||
| }, | ||
| }, | ||
| }; | ||
|
|
||
| mockSendMessage = chrome.tabs.sendMessage; | ||
| mockExecuteScript = chrome.scripting.executeScript; | ||
| mockQuery = chrome.tabs.query; | ||
|
|
||
| // Mock DOM elements | ||
| document.getElementById = jest.fn().mockImplementation(id => { | ||
| return { | ||
| addEventListener: jest.fn(), | ||
| onclick: jest.fn(), | ||
| disabled: false, | ||
| }; | ||
| }); | ||
|
|
||
| jest.clearAllMocks(); | ||
| }); | ||
|
|
||
| test('should initialize extension and log debug messages', () => { | ||
| const logSpy = jest.spyOn(console, 'log').mockImplementation(() => {}); | ||
| require('../../../src/popup/popup'); | ||
| expect(logSpy).toHaveBeenCalledWith('[POPUP_DEBUG]', 'Initializing extension...'); | ||
| logSpy.mockRestore(); | ||
| }); | ||
|
|
||
| test('should query active tab and log URL', () => { | ||
| mockQuery.mockImplementation((queryInfo, callback) => { | ||
| callback([{ url: 'https://devmcnichols.bigmachines.com/admin/configuration/rules' }]); | ||
| }); | ||
|
|
||
| require('../../../src/popup/popup'); | ||
| expect(mockQuery).toHaveBeenCalledWith({ active: true, currentWindow: true }, expect.any(Function)); | ||
| }); | ||
|
|
||
| test('should handle unload button click', () => { | ||
| const unloadButton = document.getElementById('unload'); | ||
| unloadButton.onclick(); | ||
|
|
||
| expect(mockSendMessage).toHaveBeenCalledWith( | ||
| expect.any(Number), | ||
| { greeting: 'unload' }, | ||
| expect.any(Function) | ||
| ); | ||
| }); | ||
|
|
||
| test('should handle load button click', async () => { | ||
| const loadButton = document.getElementById('load'); | ||
| const fileHandle = { | ||
| getFile: jest.fn().mockResolvedValue({ | ||
| text: jest.fn().mockResolvedValue('file contents'), | ||
| }), | ||
| }; | ||
|
|
||
| window.showOpenFilePicker = jest.fn().mockResolvedValue([fileHandle]); | ||
|
|
||
| await loadButton.addEventListener.mock.calls[0][1](); | ||
| expect(mockSendMessage).toHaveBeenCalledWith( | ||
| expect.any(Number), | ||
| { greeting: 'load', code: 'file contents' }, | ||
| expect.any(Function) | ||
| ); | ||
| }); | ||
|
|
||
| test('should set footer information on DOMContentLoaded', () => { | ||
| const footer = { innerHTML: '' }; | ||
| document.getElementById = jest.fn().mockImplementation(id => (id === 'footer' ? footer : {})); | ||
|
|
||
| document.dispatchEvent(new Event('DOMContentLoaded')); | ||
| expect(footer.innerHTML).toContain('Streamline Tools v1.0.0'); | ||
| }); | ||
| }); No newline at end of file |
Copilot
AI
Jul 3, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[nitpick] There’s significant overlap between popup.test.js and popup2.test.js. Consider consolidating these into a single test suite to reduce duplication and improve maintainability.
| /** | |
| * Unit tests for popup.js | |
| */ | |
| describe('Popup v2 Script', () => { | |
| let mockSendMessage, mockExecuteScript, mockQuery; | |
| beforeEach(() => { | |
| // Mock Chrome APIs | |
| global.chrome = { | |
| tabs: { | |
| query: jest.fn(), | |
| sendMessage: jest.fn(), | |
| }, | |
| scripting: { | |
| executeScript: jest.fn(), | |
| }, | |
| runtime: { | |
| getManifest: jest.fn(() => ({ | |
| name: 'Streamline Tools', | |
| version: '1.0.0', | |
| })), | |
| }, | |
| downloads: { | |
| setShelfEnabled: jest.fn(), | |
| onDeterminingFilename: { | |
| addListener: jest.fn(), | |
| }, | |
| }, | |
| }; | |
| mockSendMessage = chrome.tabs.sendMessage; | |
| mockExecuteScript = chrome.scripting.executeScript; | |
| mockQuery = chrome.tabs.query; | |
| // Mock DOM elements | |
| document.getElementById = jest.fn().mockImplementation(id => { | |
| return { | |
| addEventListener: jest.fn(), | |
| onclick: jest.fn(), | |
| disabled: false, | |
| }; | |
| }); | |
| jest.clearAllMocks(); | |
| }); | |
| test('should initialize extension and log debug messages', () => { | |
| const logSpy = jest.spyOn(console, 'log').mockImplementation(() => {}); | |
| require('../../../src/popup/popup'); | |
| expect(logSpy).toHaveBeenCalledWith('[POPUP_DEBUG]', 'Initializing extension...'); | |
| logSpy.mockRestore(); | |
| }); | |
| test('should query active tab and log URL', () => { | |
| mockQuery.mockImplementation((queryInfo, callback) => { | |
| callback([{ url: 'https://devmcnichols.bigmachines.com/admin/configuration/rules' }]); | |
| }); | |
| require('../../../src/popup/popup'); | |
| expect(mockQuery).toHaveBeenCalledWith({ active: true, currentWindow: true }, expect.any(Function)); | |
| }); | |
| test('should handle unload button click', () => { | |
| const unloadButton = document.getElementById('unload'); | |
| unloadButton.onclick(); | |
| expect(mockSendMessage).toHaveBeenCalledWith( | |
| expect.any(Number), | |
| { greeting: 'unload' }, | |
| expect.any(Function) | |
| ); | |
| }); | |
| test('should handle load button click', async () => { | |
| const loadButton = document.getElementById('load'); | |
| const fileHandle = { | |
| getFile: jest.fn().mockResolvedValue({ | |
| text: jest.fn().mockResolvedValue('file contents'), | |
| }), | |
| }; | |
| window.showOpenFilePicker = jest.fn().mockResolvedValue([fileHandle]); | |
| await loadButton.addEventListener.mock.calls[0][1](); | |
| expect(mockSendMessage).toHaveBeenCalledWith( | |
| expect.any(Number), | |
| { greeting: 'load', code: 'file contents' }, | |
| expect.any(Function) | |
| ); | |
| }); | |
| test('should set footer information on DOMContentLoaded', () => { | |
| const footer = { innerHTML: '' }; | |
| document.getElementById = jest.fn().mockImplementation(id => (id === 'footer' ? footer : {})); | |
| document.dispatchEvent(new Event('DOMContentLoaded')); | |
| expect(footer.innerHTML).toContain('Streamline Tools v1.0.0'); | |
| }); | |
| }); | |
| // This file has been removed after consolidating its tests into popup.test.js. |
{pr_body}