Update folv filter tests #1613
Update folv filter tests #1613
Conversation
BobrikovMikhail
changed the title
|
WalkthroughThe changes in this pull request involve updates to multiple test files and the Changes
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 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
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 26
🧹 Outside diff range and nitpick comments (14)
tests/acceptance/components/flexberry-objectlistview/filther/folv-ge-filter-test.js (2)
8-17: Improve test description and assertionsThe test description "check ge filter" is too vague. Consider providing more context about what specific aspect of the ge (greater than or equal) filter is being tested.
-executeTest('check ge filter', async (store, assert, app) => { +executeTest('should filter records with votes greater than or equal to the specified value', async (store, assert, app) => {
66-67: Improve assertion messagesThe assertion messages could be more descriptive to better indicate what's being tested.
- assert.equal(filtherResult.length >= 1, true, 'Filtered list is empty'); - assert.equal(successful, true, 'Filter successfully worked'); + assert.equal(filtherResult.length >= 1, true, 'Filtered list should contain at least one record'); + assert.equal(successful, true, 'All filtered records should have votes greater than or equal to the filter parameter');tests/acceptance/components/flexberry-objectlistview/filther/folv-le-filter-test.js (2)
23-36: Consider using more specific selectorsThe current jQuery selectors are somewhat generic. Consider using data-test-* attributes for more reliable test selectors.
-let $filterButtonDiv = $('.buttons.filter-active'); -let $objectListView = $('.object-list-view'); +let $filterButtonDiv = $('[data-test-filter-buttons]'); +let $objectListView = $('[data-test-object-list-view]');
37-45: Use consistent selector pattern for refresh buttonFor consistency with the previous suggestion, consider using a data-test attribute for the refresh button as well.
-let refreshButton = $('.refresh-button')[0]; +let refreshButton = $('[data-test-refresh-button]')[0];tests/acceptance/components/flexberry-objectlistview/filther/folv-filter-test.js (4)
7-8: Add spacing between imports and test execution for better readability.import {settled} from '@ember/test-helpers'; + executeTest('check filter', async (store, assert, app) => {
18-18: Enhance assertion message for better test documentation.- assert.equal(currentPath(), path); + assert.equal(currentPath(), path, 'Successfully transitioned to FOLV filter test page');
51-51: Remove unnecessary ESLint disable comments.The ESLint disable comments for
no-unused-varsseem unnecessary as there are no unused variables in this scope.Remove these lines as they're not serving any purpose.
Also applies to: 59-59
9-57: Consider restructuring the test for better maintainability.The test could benefit from:
- Extracting common setup into beforeEach hooks
- Using page objects pattern for better encapsulation
- Implementing proper error handling for async operations
- Adding more descriptive assertion messages
This would make the tests more maintainable and easier to understand. Would you like me to provide an example of how to implement these improvements?
tests/acceptance/components/flexberry-objectlistview/filther/folv-like-filter-test.js (1)
8-17: Enhance assertion messages for better test maintainabilityWhile the async/await conversion looks good, consider adding more descriptive assertion messages to help debug test failures.
- assert.equal(currentPath(), path); + assert.equal(currentPath(), path, `Should be on ${path} page`);tests/acceptance/components/flexberry-objectlistview/filther/folv-filter-by-enther-click-test.js (1)
28-36: Consider using data-test- attributes for selectors*Using class-based selectors for testing can be brittle if the UI changes. Consider using dedicated test attributes.
- let $filterButtonDiv = $('.buttons.filter-active'); - let $filterButton = $filterButtonDiv.children('button'); - let $objectListView = $('.object-list-view'); + let $filterButtonDiv = $('[data-test-filter-buttons]'); + let $filterButton = $('[data-test-filter-button]'); + let $objectListView = $('[data-test-object-list-view]');tests/acceptance/components/flexberry-objectlistview/filther/folv-filter-on-toolbar-test.js (2)
12-21: LGTM: Proper async/await implementationThe conversion to async/await is well done, with proper handling of the visit operation.
Consider adding a comment describing the test's purpose, especially since it's using a custom filter projection:
+// Tests filtering behavior using both common and custom filter projections executeTest('check filter on toolbar with filter projection', async (store, assert, app) => {
73-78: Consider adding explicit cleanup stepWhile the test resets the filterProjectionName, it would be beneficial to add a more comprehensive cleanup.
Consider adding a cleanup step after the test:
run(async() => { set(controller, 'filterProjectionName', undefined); -}); +}); + +// Cleanup +await settled(); +await run(() => { + controller.set('model', null); +});tests/acceptance/components/flexberry-objectlistview/filther/folv-filter-render-test.js (1)
6-115: Consider comprehensive test modernizationWhile the async/await conversion is a good step, consider a more comprehensive modernization of the test:
- Replace jQuery selectors with data-test attributes
- Use modern testing helpers (click, find, findAll)
- Remove run loops
- Update deprecated container lookups
- Consider using page objects for better test organization
This would make the tests more maintainable and resilient to UI changes.
.eslint-todo-errors.json (1)
Line range hint
1-1134: Consider implementing a systematic approach to address the linting violations.The ESLint violations fall into several categories that should be addressed systematically:
Property Access (High Priority):
- Numerous
ember/use-ember-get-and-setviolations indicate non-standard property access- Consider a codemod to automatically migrate to proper Ember getters/setters
Test Modernization (Medium Priority):
ember/no-test-and-thenviolations suggest outdated async testing patternshbs/check-hbs-template-literalsviolations in test files- Update tests to use modern
async/awaitsyntax and proper template literal checksComponent Best Practices (Medium Priority):
ember/no-incorrect-calls-with-inline-anonymous-functionsember/no-arrow-function-computed-properties- Refactor components to follow current Ember best practices
To effectively tackle these issues:
- Prioritize violations by impact and complexity
- Create automated codemods for high-frequency issues
- Update developer guidelines to prevent new violations
- Consider breaking the work into smaller, focused PRs by violation type
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (10)
- .eslint-todo-errors.json (1 hunks)
- tests/acceptance/components/flexberry-objectlistview/filther/folv-filter-by-enther-click-test.js (1 hunks)
- tests/acceptance/components/flexberry-objectlistview/filther/folv-filter-on-toolbar-test.js (1 hunks)
- tests/acceptance/components/flexberry-objectlistview/filther/folv-filter-render-test.js (1 hunks)
- tests/acceptance/components/flexberry-objectlistview/filther/folv-filter-test.js (1 hunks)
- tests/acceptance/components/flexberry-objectlistview/filther/folv-ge-filter-test.js (1 hunks)
- tests/acceptance/components/flexberry-objectlistview/filther/folv-le-filter-test.js (1 hunks)
- tests/acceptance/components/flexberry-objectlistview/filther/folv-like-filter-test.js (1 hunks)
- tests/acceptance/components/flexberry-objectlistview/filther/folv-neq-filter-test.js (1 hunks)
- tests/acceptance/components/flexberry-objectlistview/filther/folv-without-operation-filter-test.js (1 hunks)
🧰 Additional context used
🔇 Additional comments (11)
tests/acceptance/components/flexberry-objectlistview/filther/folv-neq-filter-test.js (1)
8-17: LGTM: Test setup follows async/await best practicesThe conversion to async/await improves readability and properly handles asynchronous navigation.
tests/acceptance/components/flexberry-objectlistview/filther/folv-le-filter-test.js (1)
6-12: LGTM: Test setup properly converted to async/awaitThe import of
settledand conversion to async/await pattern is appropriate for handling asynchronous operations in the test.tests/acceptance/components/flexberry-objectlistview/filther/folv-filter-test.js (1)
19-23: Verify data preparation edge cases.The current implementation has several potential issues:
- No validation if the query returns empty results
- Using undefined values in
filtreInsertValueArrcould lead to unexpected behavior- Direct array access without bounds checking
tests/acceptance/components/flexberry-objectlistview/filther/folv-without-operation-filter-test.js (2)
8-14: LGTM: Clean async conversion and setupThe conversion to async/await and the initial test setup are well-structured.
15-26: 🛠️ Refactor suggestionImprove data setup robustness
The current data setup has several potential issues:
- The slice operation on line 22 could throw if address is empty
- The error message "Empty data" could be more descriptive
- The query assumes data exists without proper error handling
Let's verify if empty addresses are possible:
Consider adding proper error handling:
- let arr = result.toArray(); - filtreInsertParametr = arr.objectAt(0).get('address'); - filtreInsertParametr = filtreInsertParametr.slice(1, filtreInsertParametr.length); - if (!filtreInsertParametr) { - assert.ok(false, 'Empty data'); - } + let arr = result.toArray(); + if (arr.length === 0) { + throw new Error('No test data found in store'); + } + let address = arr.objectAt(0).get('address'); + if (!address) { + throw new Error('Test requires non-empty address field'); + } + filtreInsertParametr = address.slice(1);tests/acceptance/components/flexberry-objectlistview/filther/folv-like-filter-test.js (1)
39-48: 🛠️ Refactor suggestionImprove refresh mechanism reliability
The refresh button click might need to ensure the button is rendered and interactive.
let refreshFunction = async function () { - let refreshButton = $('.refresh-button')[0]; - refreshButton.click(); + await settled(); + let refreshButton = $('.refresh-button'); + assert.ok(refreshButton.length, 'Refresh button should be present'); + await click('.refresh-button'); };✅ Verification successful
Let me try a different search pattern to gather more context about refresh button usage in tests.
These commands will help us:
- Find refresh button usage with surrounding context
- Locate similar refresh list patterns
- Identify refresh-related helper functions
This will give us better insight into the existing refresh patterns and testing practices in the codebase.
Based on the search results, I can now provide a final response. The codebase shows multiple examples of refresh button handling, with one test file (folv-dropdown-filter-for-directories-test.js) already implementing the suggested pattern using
settled()and proper button selection:const refreshButton = document.querySelector('.refresh-button'); await click(refreshButton); await settled(); // Wait for the refresh to processThe review suggestion is valid and matches existing patterns
The proposed improvement to use
settled()and proper assertions is consistent with best practices found in the codebase. The suggested changes will make the test more reliable by:
- Ensuring the button is present before clicking
- Using the test helper
click()instead of direct DOM manipulation- Waiting for the refresh operation to complete with
settled()The refactoring suggestion in the review comment is appropriate and should be implemented.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check if similar refresh patterns exist in other test files rg -l "refresh-button.*click\(\)" tests/acceptance/Length of output: 53
Script:
#!/bin/bash # Search for refresh button related patterns with broader context rg -A 5 "refresh.*button" tests/acceptance/ # Also check for similar refresh patterns rg -A 5 "refreshList" tests/acceptance/ # Look for test helper functions related to refresh ast-grep --pattern 'function refresh$_($$$) { $$$ }'Length of output: 57697
tests/acceptance/components/flexberry-objectlistview/filther/folv-filter-by-enther-click-test.js (2)
7-7: LGTM: Proper async/await setupThe test setup correctly implements async/await pattern and includes necessary test helpers.
Also applies to: 9-13
38-40: Verify successful filter applicationConsider adding a verification step after filter application to ensure it was successful.
tests/acceptance/components/flexberry-objectlistview/filther/folv-filter-on-toolbar-test.js (2)
10-10: LGTM: Good addition of settled helperThe addition of
settledfrom @ember/test-helpers is appropriate for ensuring async operations complete before assertions.
22-33: Address TODO comment regarding predicate limitationThe TODO comment indicates a missing feature in the query predicate system. This should be tracked and addressed.
Would you like me to create a GitHub issue to track the implementation of the "address != type.name" predicate feature?
Let's check if this limitation is documented or if there are related issues:
tests/acceptance/components/flexberry-objectlistview/filther/folv-filter-render-test.js (1)
6-12: LGTM: Proper async/await conversionThe conversion to async/await syntax is well implemented, with the visit operation correctly awaited.
| let $filterButtonDiv = $('.buttons.filter-active'); | ||
| let $filterButton = $filterButtonDiv.children('button'); | ||
| let $objectListView = $('.object-list-view'); | ||
|
|
||
| // Activate filtre row. | ||
| await run(async () => { | ||
| $filterButton.click(); | ||
| }); | ||
|
|
||
| await run(async () => { | ||
| await filterCollumn($objectListView, 0, filtreInsertOperation, filtreInsertParametr); | ||
| }); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Simplify run loop and selector usage
While the async handling is correct, the code structure could be improved.
Consider this simplified version:
- let $filterButtonDiv = $('.buttons.filter-active');
- let $filterButton = $filterButtonDiv.children('button');
- let $objectListView = $('.object-list-view');
-
- // Activate filtre row.
- await run(async () => {
- $filterButton.click();
- });
-
- await run(async () => {
- await filterCollumn($objectListView, 0, filtreInsertOperation, filtreInsertParametr);
- });
+ const selectors = {
+ filterButton: '.buttons.filter-active button',
+ objectListView: '.object-list-view'
+ };
+
+ // Activate filter row and apply filter
+ await run(async () => {
+ $(selectors.filterButton).click();
+ await filterCollumn(
+ $(selectors.objectListView),
+ 0,
+ filtreInsertOperation,
+ filtreInsertParametr
+ );
+ });📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| let $filterButtonDiv = $('.buttons.filter-active'); | |
| let $filterButton = $filterButtonDiv.children('button'); | |
| let $objectListView = $('.object-list-view'); | |
| // Activate filtre row. | |
| await run(async () => { | |
| $filterButton.click(); | |
| }); | |
| await run(async () => { | |
| await filterCollumn($objectListView, 0, filtreInsertOperation, filtreInsertParametr); | |
| }); | |
| const selectors = { | |
| filterButton: '.buttons.filter-active button', | |
| objectListView: '.object-list-view' | |
| }; | |
| // Activate filter row and apply filter | |
| await run(async () => { | |
| $(selectors.filterButton).click(); | |
| await filterCollumn( | |
| $(selectors.objectListView), | |
| 0, | |
| filtreInsertOperation, | |
| filtreInsertParametr | |
| ); | |
| }); |
| let builder2 = new Builder(store).from(modelName).selectByProjection('SuggestionL').where('address', FilterOperator.Neq, '').top(1); | ||
| let result = await store.query(modelName, builder2.build()); | ||
| let arr = result.toArray(); | ||
| filtreInsertParametr = arr.objectAt(0).get('address'); | ||
| if (!filtreInsertParametr) { | ||
| assert.ok(false, 'Empty data'); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Enhance data preparation robustness
The current data preparation could be more robust:
- The error message should be more descriptive
- The query results should be validated more thoroughly
- Error handling should be added for the query operation
Consider this improvement:
- let builder2 = new Builder(store).from(modelName).selectByProjection('SuggestionL').where('address', FilterOperator.Neq, '').top(1);
- let result = await store.query(modelName, builder2.build());
- let arr = result.toArray();
- filtreInsertParametr = arr.objectAt(0).get('address');
- if (!filtreInsertParametr) {
- assert.ok(false, 'Empty data');
- }
+ try {
+ let builder2 = new Builder(store).from(modelName)
+ .selectByProjection('SuggestionL')
+ .where('address', FilterOperator.Neq, '')
+ .top(1);
+ let result = await store.query(modelName, builder2.build());
+ let arr = result.toArray();
+
+ assert.ok(arr.length > 0, 'Query should return at least one record for testing');
+
+ filtreInsertParametr = arr.objectAt(0).get('address');
+ assert.ok(
+ filtreInsertParametr,
+ 'Test data validation failed: address field is required for neq filter test'
+ );
+ } catch (error) {
+ assert.ok(false, `Failed to prepare test data: ${error.message}`);
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| let builder2 = new Builder(store).from(modelName).selectByProjection('SuggestionL').where('address', FilterOperator.Neq, '').top(1); | |
| let result = await store.query(modelName, builder2.build()); | |
| let arr = result.toArray(); | |
| filtreInsertParametr = arr.objectAt(0).get('address'); | |
| if (!filtreInsertParametr) { | |
| assert.ok(false, 'Empty data'); | |
| } | |
| try { | |
| let builder2 = new Builder(store).from(modelName) | |
| .selectByProjection('SuggestionL') | |
| .where('address', FilterOperator.Neq, '') | |
| .top(1); | |
| let result = await store.query(modelName, builder2.build()); | |
| let arr = result.toArray(); | |
| assert.ok(arr.length > 0, 'Query should return at least one record for testing'); | |
| filtreInsertParametr = arr.objectAt(0).get('address'); | |
| assert.ok( | |
| filtreInsertParametr, | |
| 'Test data validation failed: address field is required for neq filter test' | |
| ); | |
| } catch (error) { | |
| assert.ok(false, `Failed to prepare test data: ${error.message}`); | |
| } |
| // Apply filter function. | ||
| let refreshFunction = async function () { | ||
| let refreshButton = $('.refresh-button')[0]; | ||
| refreshButton.click(); | ||
| }; | ||
|
|
||
| // Apply filter. | ||
| let controller = app.__container__.lookup('controller:' + currentRouteName()); | ||
|
|
||
| await refreshListByFunction(refreshFunction, controller); | ||
| let filtherResult = controller.model.content; | ||
| let successful = true; | ||
| for (let i = 0; i < filtherResult.length; i++) { | ||
| let address = filtherResult[i]._data.address; | ||
| if (address === filtreInsertParametr) { | ||
| successful = false; | ||
| } | ||
| } | ||
|
|
||
| assert.equal(successful, true, 'Filter successfully worked'); | ||
| }); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Improve filter verification logic
The current verification approach could be more maintainable and robust.
Consider this improved implementation:
- let refreshFunction = async function () {
- let refreshButton = $('.refresh-button')[0];
- refreshButton.click();
- };
-
- // Apply filter.
- let controller = app.__container__.lookup('controller:' + currentRouteName());
-
- await refreshListByFunction(refreshFunction, controller);
- let filtherResult = controller.model.content;
- let successful = true;
- for (let i = 0; i < filtherResult.length; i++) {
- let address = filtherResult[i]._data.address;
- if (address === filtreInsertParametr) {
- successful = false;
- }
- }
-
- assert.equal(successful, true, 'Filter successfully worked');
+ const refreshFunction = async () => {
+ $('.refresh-button')[0].click();
+ };
+
+ const controller = app.__container__.lookup('controller:' + currentRouteName());
+ await refreshListByFunction(refreshFunction, controller);
+
+ const filterResults = controller.model.content;
+ assert.ok(filterResults.length > 0, 'Filter should return results');
+
+ const hasMatchingAddress = filterResults.some(record => {
+ const address = record._data.address;
+ return typeof address === 'string' && address === filtreInsertParametr;
+ });
+
+ assert.notOk(
+ hasMatchingAddress,
+ `Filter should exclude records with address "${filtreInsertParametr}"`
+ );📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // Apply filter function. | |
| let refreshFunction = async function () { | |
| let refreshButton = $('.refresh-button')[0]; | |
| refreshButton.click(); | |
| }; | |
| // Apply filter. | |
| let controller = app.__container__.lookup('controller:' + currentRouteName()); | |
| await refreshListByFunction(refreshFunction, controller); | |
| let filtherResult = controller.model.content; | |
| let successful = true; | |
| for (let i = 0; i < filtherResult.length; i++) { | |
| let address = filtherResult[i]._data.address; | |
| if (address === filtreInsertParametr) { | |
| successful = false; | |
| } | |
| } | |
| assert.equal(successful, true, 'Filter successfully worked'); | |
| }); | |
| const refreshFunction = async () => { | |
| $('.refresh-button')[0].click(); | |
| }; | |
| const controller = app.__container__.lookup('controller:' + currentRouteName()); | |
| await refreshListByFunction(refreshFunction, controller); | |
| const filterResults = controller.model.content; | |
| assert.ok(filterResults.length > 0, 'Filter should return results'); | |
| const hasMatchingAddress = filterResults.some(record => { | |
| const address = record._data.address; | |
| return typeof address === 'string' && address === filtreInsertParametr; | |
| }); | |
| assert.notOk( | |
| hasMatchingAddress, | |
| `Filter should exclude records with address "${filtreInsertParametr}"` | |
| ); | |
| }); |
| await run(async() => { | ||
| await filterCollumn($objectListView, 2, filtreInsertOperation, filtreInsertParametr); | ||
| }); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Simplify nested async/await structure
The nested run and async calls can be simplified.
- await run(async() => {
- await filterCollumn($objectListView, 2, filtreInsertOperation, filtreInsertParametr);
- });
+ await filterCollumn($objectListView, 2, filtreInsertOperation, filtreInsertParametr);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| await run(async() => { | |
| await filterCollumn($objectListView, 2, filtreInsertOperation, filtreInsertParametr); | |
| }); | |
| await filterCollumn($objectListView, 2, filtreInsertOperation, filtreInsertParametr); |
| let refreshFunction = async function () { | ||
| run(() => { | ||
| let refreshButton = $('.refresh-button')[0]; | ||
| refreshButton.click(); | ||
| }); | ||
| }; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Avoid nested run loop in async function
The refresh function contains a nested run loop which is unnecessary in an async function.
- let refreshFunction = async function () {
- run(() => {
- let refreshButton = $('.refresh-button')[0];
- refreshButton.click();
- });
- };
+ let refreshFunction = async function () {
+ $('.refresh-button')[0].click();
+ await settled();
+ };📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| let refreshFunction = async function () { | |
| run(() => { | |
| let refreshButton = $('.refresh-button')[0]; | |
| refreshButton.click(); | |
| }); | |
| }; | |
| let refreshFunction = async function () { | |
| $('.refresh-button')[0].click(); | |
| await settled(); | |
| }; |
| let $filterButtonDiv = $('.buttons.filter-active'); | ||
| let $filterButton = $filterButtonDiv.children('button'); | ||
| let $filterRemoveButton = $filterButtonDiv.children('.removeFilter-button'); | ||
| let $filterButtonIcon = $filterButton.children('i'); | ||
|
|
||
| const $table = $('.object-list-view'); | ||
| const $tableTbody = $table.children('tbody'); | ||
| let $tableRows = $tableTbody.children('tr'); | ||
|
|
||
| // Check filtre button div. | ||
| assert.strictEqual($filterButtonDiv.prop('tagName'), 'DIV', 'Filtre button\'s wrapper is a <div>'); | ||
| assert.strictEqual($filterButtonDiv.hasClass('ui icon buttons'), true, 'Filtre button\'s wrapper has \'ui icon buttons\' css-class'); | ||
| assert.strictEqual($filterButtonDiv.hasClass('filter-active'), true, 'Filtre button\'s wrapper has \'filter-active\' css-class'); | ||
| assert.strictEqual($filterButtonDiv.length === 1, true, 'Component has filter button'); | ||
|
|
||
| // Check filtre button. | ||
| assert.strictEqual($filterButton.length === 1, true, 'Filtre button has inner button block'); | ||
| assert.strictEqual($filterButton.hasClass('ui button'), true, 'Filtre button\'s wrapper has \'ui button\' css-class'); | ||
| assert.strictEqual($filterButton[0].title, 'Добавить фильтр', 'Filtre button has title'); | ||
| assert.strictEqual($filterButton.prop('tagName'), 'BUTTON', 'Component\'s inner button block is a <button>'); | ||
|
|
||
| // Check button's icon <i>. | ||
| assert.strictEqual($filterButtonIcon.length === 1, true, 'Filtre button\'s title has icon block'); | ||
| assert.strictEqual($filterButtonIcon.prop('tagName'), 'I', 'Filtre button\'s icon block is a <i>'); | ||
| assert.strictEqual($filterButtonIcon.hasClass('filter icon'), true, 'Filtre button\'s icon block has \'filter icon\' css-class'); | ||
|
|
||
| // Check filtre remove button. | ||
| assert.strictEqual($filterRemoveButton.length === 0, true, 'Component hasn\'t remove filter button'); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider using modern test selectors
While the current jQuery selectors work, consider using data-test-* attributes with modern testing helpers for more maintainable and resilient tests.
Example refactor:
-let $filterButtonDiv = $('.buttons.filter-active');
+let $filterButtonDiv = find('[data-test-filter-button-wrapper]');And in your component template:
| let controller = app.__container__.lookup('controller:' + currentRouteName()); | ||
| await refreshListByFunction(refreshFunction, controller); |
There was a problem hiding this comment.
Update deprecated container lookup
The __container__ syntax is deprecated. Use the owner API instead.
Replace with:
-let controller = app.__container__.lookup('controller:' + currentRouteName());
+let controller = app.owner.lookup(`controller:${currentRouteName()}`);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| let controller = app.__container__.lookup('controller:' + currentRouteName()); | |
| await refreshListByFunction(refreshFunction, controller); | |
| let controller = app.owner.lookup(`controller:${currentRouteName()}`); | |
| await refreshListByFunction(refreshFunction, controller); |
| await run(async() => { | ||
| $filterButton.click(); | ||
| }); |
There was a problem hiding this comment.
Remove unnecessary run loop wrapper
When using async/await, the run wrapper is unnecessary. Consider using modern testing helpers.
Replace with:
-await run(async() => {
- $filterButton.click();
-});
+await click('[data-test-filter-button]');Committable suggestion was skipped due to low confidence.
| await run(async() => { | ||
| $filterButton.click(); | ||
| }); |
There was a problem hiding this comment.
Remove unnecessary run loop wrapper
Similar to the previous run loop usage, this can be simplified with modern testing helpers.
Replace with:
-await run(async() => {
- $filterButton.click();
-});
+await click('[data-test-filter-button]');Committable suggestion was skipped due to low confidence.
| { "ruleId": "ember/use-ember-get-and-set", "line": 22, "column": 42 } | ||
| ], | ||
| "tests/acceptance/components/flexberry-objectlistview/filther/folv-filter-on-toolbar-test.js": [ | ||
| "tests/acceptance/components/flexberry-objectlistview/filther/folv-filter-on-toolbar-test.js": [ | ||
| { "ruleId": "ember/no-test-import-export", "line": 4, "column": 1 }, | ||
| { "ruleId": "ember/no-test-and-then", "line": 19, "column": 3 }, | ||
| { "ruleId": "ember/use-ember-get-and-set", "line": 28, "column": 39 }, | ||
| { "ruleId": "ember/use-ember-get-and-set", "line": 29, "column": 41 }, | ||
| { "ruleId": "ember/no-test-and-then", "line": 40, "column": 7 }, | ||
| { "ruleId": "no-unused-vars", "line": 49, "column": 66 }, | ||
| { "ruleId": "no-unused-vars", "line": 64, "column": 68 }, | ||
| { "ruleId": "no-unused-vars", "line": 75, "column": 70 } | ||
| ], | ||
| "tests/acceptance/components/flexberry-objectlistview/filther/folv-filter-render-test.js": [ | ||
| { "ruleId": "ember/no-test-import-export", "line": 3, "column": 1 }, | ||
| { "ruleId": "ember/no-test-and-then", "line": 12, "column": 5 } | ||
| { "ruleId": "ember/use-ember-get-and-set", "line": 29, "column": 35 }, | ||
| { "ruleId": "ember/use-ember-get-and-set", "line": 30, "column": 37 } | ||
| ], | ||
| "tests/acceptance/components/flexberry-objectlistview/filther/folv-filter-render-test.js": [ | ||
| { "ruleId": "ember/no-test-import-export", "line": 3, "column": 1 } | ||
| ], | ||
| "tests/acceptance/components/flexberry-objectlistview/filther/folv-filter-test.js": [ | ||
| { "ruleId": "ember/no-test-import-export", "line": 2, "column": 1 }, | ||
| { "ruleId": "ember/no-test-and-then", "line": 16, "column": 3 }, | ||
| { "ruleId": "ember/use-ember-get-and-set", "line": 21, "column": 47 }, | ||
| { "ruleId": "ember/use-ember-get-and-set", "line": 21, "column": 90 }, | ||
| { "ruleId": "ember/use-ember-get-and-set", "line": 22, "column": 23 }, | ||
| { "ruleId": "ember/use-ember-get-and-set", "line": 22, "column": 68 }, | ||
| { "ruleId": "ember/use-ember-get-and-set", "line": 22, "column": 102 } | ||
| { "ruleId": "ember/use-ember-get-and-set", "line": 22, "column": 43 }, | ||
| { "ruleId": "ember/use-ember-get-and-set", "line": 22, "column": 86 }, | ||
| { "ruleId": "ember/use-ember-get-and-set", "line": 23, "column": 19 }, | ||
| { "ruleId": "ember/use-ember-get-and-set", "line": 23, "column": 64 }, | ||
| { "ruleId": "ember/use-ember-get-and-set", "line": 23, "column": 98 } | ||
| ], | ||
| "tests/acceptance/components/flexberry-objectlistview/filther/folv-ge-filter-test.js": [ | ||
| "tests/acceptance/components/flexberry-objectlistview/filther/folv-ge-filter-test.js": [ | ||
| { "ruleId": "ember/no-test-import-export", "line": 2, "column": 1 }, | ||
| { "ruleId": "ember/no-test-and-then", "line": 15, "column": 3 }, | ||
| { "ruleId": "ember/use-ember-get-and-set", "line": 20, "column": 46 } | ||
| { "ruleId": "ember/use-ember-get-and-set", "line": 21, "column": 42 } | ||
| ], | ||
| "tests/acceptance/components/flexberry-objectlistview/filther/folv-le-filter-test.js": [ | ||
| { "ruleId": "ember/no-test-import-export", "line": 2, "column": 1 }, | ||
| { "ruleId": "ember/no-test-and-then", "line": 15, "column": 3 }, | ||
| { "ruleId": "ember/use-ember-get-and-set", "line": 20, "column": 46 } | ||
| { "ruleId": "ember/use-ember-get-and-set", "line": 21, "column": 43 } | ||
| ], | ||
| "tests/acceptance/components/flexberry-objectlistview/filther/folv-like-filter-test.js": [ | ||
| { "ruleId": "ember/no-test-import-export", "line": 2, "column": 1 }, | ||
| { "ruleId": "ember/no-test-and-then", "line": 16, "column": 3 }, | ||
| { "ruleId": "ember/use-ember-get-and-set", "line": 21, "column": 46 } | ||
| { "ruleId": "ember/use-ember-get-and-set", "line": 21, "column": 42 } | ||
| ], | ||
| "tests/acceptance/components/flexberry-objectlistview/filther/folv-neq-filter-test.js": [ | ||
| "tests/acceptance/components/flexberry-objectlistview/filther/folv-neq-filter-test.js": [ | ||
| { "ruleId": "ember/no-test-import-export", "line": 2, "column": 1 }, | ||
| { "ruleId": "ember/no-test-and-then", "line": 16, "column": 3 }, | ||
| { "ruleId": "ember/use-ember-get-and-set", "line": 21, "column": 46 } | ||
| { "ruleId": "ember/use-ember-get-and-set", "line": 21, "column": 42 } | ||
| ], | ||
| "tests/acceptance/components/flexberry-objectlistview/filther/folv-without-operation-filter-test.js": [ | ||
| { "ruleId": "ember/no-test-import-export", "line": 2, "column": 1 }, | ||
| { "ruleId": "ember/no-test-and-then", "line": 16, "column": 3 }, | ||
| { "ruleId": "ember/use-ember-get-and-set", "line": 21, "column": 46 } | ||
| { "ruleId": "ember/use-ember-get-and-set", "line": 21, "column": 42 } |
There was a problem hiding this comment.
New test files should follow modern Ember practices.
The newly added entries for FOLV filter tests contain ember/use-ember-get-and-set violations. Rather than adding these to the allowlist, consider:
- Using native property access or proper Ember getters/setters
- Setting an example for new code by following current best practices
- Avoiding the accumulation of additional technical debt
Instead of adding these to .eslint-todo-errors.json, fix the violations in the new test files:
- ember/use-ember-get-and-set", "line": 22, "column": 42
+ // Use proper Ember getters/setters or native property access
+ // Example:
+ // Instead of: this.get('property')
+ // Use: this.propertyCommittable suggestion was skipped due to low confidence.
Summary by CodeRabbit
Bug Fixes
New Features
Documentation