vercel[bot]
commented
2 months ago The latest updates on your projects. Learn more about Vercel for Git ↗︎
| Name | Status | Preview | Updated (UTC) |
|---|---|---|---|
| mr-market | ✅ Ready (Inspect) | Visit Preview | Apr 15, 2024 8:42am |
Closed zed-wong closed 2 months ago
vercel[bot]
commented
2 months ago The latest updates on your projects. Learn more about Vercel for Git ↗︎
| Name | Status | Preview | Updated (UTC) |
|---|---|---|---|
| mr-market | ✅ Ready (Inspect) | Visit Preview | Apr 15, 2024 8:42am |
railway-app[bot]
commented
2 months ago This PR is being deployed to Railway 🚅
railway-app[bot]
commented
2 months ago This PR is being deployed to Railway 🚅
Mr.Market: ◻️ REMOVED
github-actions[bot]
commented
2 months ago PR Description updated to latest commit (https://github.com/Hu-Fi/Mr.Market/commit/ec6585447e9ac54d2ae6b2e467ea6d1aef813252)
github-actions[bot]
commented
2 months ago | ⏱️ Estimated effort to review [1-5] | 3, because the PR includes multiple test files and updates to localization files, which requires careful review to ensure that all new features are covered by tests and that the localizations are correctly implemented. |
| 🧪 Relevant tests | Yes |
| 🔍 Possible issues | Possible Bug: The removal of 'binance' from the list of supported exchanges in `coin.test.ts` might affect existing functionalities that rely on Binance as a supported exchange. This change should be validated to ensure it does not break any existing features. |
| 🔒 Security concerns | No |
| relevant file | interface/src/lib/helpers/hufi/coin.test.ts |
| suggestion | Consider adding a test case to verify that the removal of 'binance' from the supported exchanges does not negatively impact other functionalities that might rely on this exchange. This is important to ensure that existing features remain stable after this change. [important] |
| relevant line | const exchanges: SupportedExchanges[] = ['bitfinex', 'mexc'] |
| relevant file | interface/tests/grow/arbitrage.spec.ts |
| suggestion | It's recommended to add error handling for network requests or unexpected user input during the arbitrage creation process. This could improve the robustness of the test by ensuring it can gracefully handle errors. [important] |
| relevant line | await page.getByTestId('create-new-arbitrage').click(); |
| relevant file | interface/src/lib/components/dialogs/grow/arbitrage/confirmPayment.svelte |
| suggestion | Implement a check to disable the payment button if the payment has already been made. This prevents users from making duplicate payments and enhances the user experience. [important] |
| relevant line | data-testid='pay-btn-1' |
| relevant file | interface/src/lib/components/grow/arbitrage/new/easy/exchanges.svelte |
| suggestion | Add a confirmation dialog or visual feedback when an exchange is selected. This would improve user interaction by providing clear feedback that their selection has been registered. [medium] |
| relevant line | data-testid="select-exchange-1" |
github-actions[bot]
commented
2 months ago | Category | Suggestions |
| Maintainability |
Replace hard-coded exchange list with a dynamic import from a configuration file.___ **Replace the hard-coded array of exchanges with a dynamic import or configuration to makethe test more flexible and maintainable.** [interface/src/lib/helpers/hufi/coin.test.ts [20]](https://github.com/Hu-Fi/Mr.Market/pull/150/files#diff-c4b1b5ad4260f0ce5038c8533d8b409591a42749d3beb2a057c7d9c9eed6774cR20-R20) ```diff -const exchanges: SupportedExchanges[] = ['bitfinex', 'mexc'] +import { supportedExchanges } from './config'; +const exchanges: SupportedExchanges[] = supportedExchanges; ``` |
Extract repetitive button styling into a shared CSS class for better maintainability.___ **Extract the button styling into a shared CSS class to reduce inline styling duplicationand improve maintainability.** [interface/src/lib/components/dialogs/grow/arbitrage/confirmPayment.svelte [121-127]](https://github.com/Hu-Fi/Mr.Market/pull/150/files#diff-94efc6491a5ac4ba49c323d40db203e28413357471538d2f387436823a658dfeR121-R127) ```diff + + | |
Remove unused attributes from SVG elements.___ **Remove the unusedxmlns attribute from the script tag within the SVG element to clean up the code and avoid potential confusion.** [interface/src/lib/components/grow/elements/entrance.svelte [36]](https://github.com/Hu-Fi/Mr.Market/pull/150/files#diff-9dd438341b4932b318db300bf202ffda6e31919d222daae0f0c96e69117e6621R36-R36) ```diff - + ``` | |
Use responsive CSS classes for dynamic height management.___ **Replace the hardcoded height values with a more responsive approach using CSS classes orstyles that adapt based on the viewport or container size.** [interface/src/lib/components/spot/order/elements/orders.svelte [116]](https://github.com/Hu-Fi/Mr.Market/pull/150/files#diff-04d4c3c85dccb794dc6cd0e4b7a1b660d1a9ae9c5a0e45e3ce7fd5b39034529eR116-R116) ```diff -
+
```
| |
| Enhancement |
Use a loop to automate repetitive test actions for selecting exchanges.___ **Use a loop to automate repetitive test actions and reduce code duplication for clickingand filling actions.** [interface/tests/grow/arbitrage.spec.ts [18-22]](https://github.com/Hu-Fi/Mr.Market/pull/150/files#diff-b7cac63a8a5df2eaac47dd8b56920083c7d7e09aa4dbbd8fea8602170a312c28R18-R22) ```diff -await page.getByTestId('select-exchange-1').click(); -await page.getByTestId('1-exchange-0').click(); -await page.getByTestId('select-exchange-2').click(); -await page.getByTestId('2-exchange-1').click(); +const exchanges = ['1-exchange-0', '2-exchange-1']; +for (const exchange of exchanges) { + await page.getByTestId(`select-exchange-${exchange.charAt(0)}`).click(); + await page.getByTestId(exchange).click(); +} ``` |
Combine duplicate class attributes into a single attribute.___ **Avoid using duplicate class attributes in HTML elements. Combine the class definitionsinto a single attribute to ensure proper behavior and avoid potential conflicts.** [interface/src/lib/components/dialogs/spotPairSelector.svelte [51]](https://github.com/Hu-Fi/Mr.Market/pull/150/files#diff-95b123936815b24a8e451e9577a6a1ca76a14a6a1022a21c50f335f142bf123eR51-R51) ```diff - | |
Add an
___
**Consider adding an | |
| Reliability |
Add error handling for page navigation to enhance test reliability.___ **Implement error handling for navigation and interaction methods to improve testreliability.** [interface/tests/grow/arbitrage.spec.ts [8]](https://github.com/Hu-Fi/Mr.Market/pull/150/files#diff-b7cac63a8a5df2eaac47dd8b56920083c7d7e09aa4dbbd8fea8602170a312c28R8-R8) ```diff -await page.goto('http://127.0.0.1:5173/grow'); +try { + await page.goto('http://127.0.0.1:5173/grow'); +} catch (error) { + console.error('Navigation to grow page failed:', error); +} ``` |
| Best practice |
Refactor duplicated viewport configuration into a shared function for better code reuse.___ **Refactor duplicated viewport configuration into a shared setup function to improve codereuse.** [interface/tests/grow/market-making.spec.ts [4]](https://github.com/Hu-Fi/Mr.Market/pull/150/files#diff-928b43985cc21ae40ff882f87b76d8a88f7161f9531de8faddf316d04dd54661R4-R4) ```diff -test.use({ - viewport: { width: 390, height: 844 }, // iPhone 14 Pro +// In a separate file or a setup function +export const setupViewport = () => ({ + viewport: { width: 390, height: 844 } }); +// Usage in test files +test.use(setupViewport()); + ``` |
Replace generic array creation with a specific array for clarity.___ **Use a more specific method thanArray(2) to generate arrays when the size and content are known, to improve code clarity and prevent errors in array handling.** [interface/src/lib/components/grow/arbitrage/new/easy/exchanges.svelte [218]](https://github.com/Hu-Fi/Mr.Market/pull/150/files#diff-76a6a96298f7905c2da1e23483669b1951f5de3e0b25f37df050d78eb0878233R218-R218) ```diff -{#each Array(2) as _, i} +{#each ['item1', 'item2'] as item, i} ``` | |
Use named functions for event handlers to enhance code readability and reusability.___ **Replace the inline function in theon:click event with a named function to improve readability and reusability of the code.** [interface/src/lib/components/dialogs/spotPairSelector.svelte [58]](https://github.com/Hu-Fi/Mr.Market/pull/150/files#diff-95b123936815b24a8e451e9577a6a1ca76a14a6a1022a21c50f335f142bf123eR58-R58) ```diff - + + ``` | |
| Accessibility |
Add descriptive text to the
___
**Ensure that the |
Use a more descriptive
___
**Consider using a more descriptive | |
| Performance |
Add a key attribute to list items for better reactivity and performance.___ **Consider adding a key attribute to each item in the list for better performance andpredictable behavior during updates.** [interface/src/lib/components/grow/marketMaking/new/easy/pair.svelte [111]](https://github.com/Hu-Fi/Mr.Market/pull/150/files#diff-a2d0e23cb6e36f9f288d2e124d3e9d8de7161761be5a386a5101355ada212a4fR111-R111) ```diff -{#each marketMakingPairs as item, i} +{#each marketMakingPairs as item, i (item.id)} ``` |
github-actions[bot]
commented
2 months ago PR Description updated to latest commit (https://github.com/Hu-Fi/Mr.Market/commit/ec6585447e9ac54d2ae6b2e467ea6d1aef813252)
github-actions[bot]
commented
2 months ago Persistent review updated to latest commit https://github.com/Hu-Fi/Mr.Market/commit/ec6585447e9ac54d2ae6b2e467ea6d1aef813252
github-actions[bot]
commented
2 months ago | Category | Suggestions |
| Possible issue |
Add 'binance' to the exchanges array if its removal was unintentional.___ **Consider adding 'binance' back to theexchanges array if it was removed by mistake. If the removal was intentional, ensure that all dependent tests and functionalities are updated accordingly.** [interface/src/lib/helpers/hufi/coin.test.ts [20]](https://github.com/Hu-Fi/Mr.Market/pull/150/files#diff-c4b1b5ad4260f0ce5038c8533d8b409591a42749d3beb2a057c7d9c9eed6774cR20-R20) ```diff -const exchanges: SupportedExchanges[] = ['bitfinex', 'mexc'] +const exchanges: SupportedExchanges[] = ['binance', 'bitfinex', 'mexc'] ``` |
| Maintainability |
Refactor repetitive test actions into loops or functions for better maintainability.___ **Consider using a loop or a function to automate the repetitive test actions for selectingexchanges and trading pairs, which will make the code cleaner and more maintainable.** [interface/tests/grow/arbitrage.spec.ts [18-25]](https://github.com/Hu-Fi/Mr.Market/pull/150/files#diff-b7cac63a8a5df2eaac47dd8b56920083c7d7e09aa4dbbd8fea8602170a312c28R18-R25) ```diff -await page.getByTestId('select-exchange-1').click(); -await page.getByTestId('1-exchange-0').click(); -await page.getByTestId('select-exchange-2').click(); -await page.getByTestId('2-exchange-1').click(); -await page.getByTestId('select-trading-pair').click(); -await page.getByTestId('pair-0').click(); +const exchanges = ['1-exchange-0', '2-exchange-1']; +const tradingPairs = ['pair-0']; +for (const exchange of exchanges) { + await page.getByTestId('select-exchange').click(); + await page.getByTestId(exchange).click(); +} +for (const pair of tradingPairs) { + await page.getByTestId('select-trading-pair').click(); + await page.getByTestId(pair).click(); +} ``` |
Use a function to handle string splitting for robustness.___ **Consider using a more specific method than.split("/") directly in the template to handle potential errors or edge cases where the expected format might not be met.** [interface/src/lib/components/grow/arbitrage/new/easy/exchanges.svelte [227]](https://github.com/Hu-Fi/Mr.Market/pull/150/files#diff-76a6a96298f7905c2da1e23483669b1951f5de3e0b25f37df050d78eb0878233R227-R227) ```diff -{$createArbPair.split("/")[i]} +{getCurrencyName($createArbPair, i)} ``` | |
Replace repeated SVGs with a reusable component.___ **Consolidate the repeated SVG elements into a reusable Svelte component to reduceredundancy and improve maintainability.** [interface/src/lib/components/grow/elements/entrance.svelte [28-36]](https://github.com/Hu-Fi/Mr.Market/pull/150/files#diff-9dd438341b4932b318db300bf202ffda6e31919d222daae0f0c96e69117e6621R28-R36) ```diff - - - + | |
Use a loop to generate repetitive HTML structures.___ **Use a loop to generate repetitive HTML structures to simplify the template and enhancereadability.** [interface/src/lib/components/grow/arbitrage/new/easy/exchanges.svelte [219-220]](https://github.com/Hu-Fi/Mr.Market/pull/150/files#diff-76a6a96298f7905c2da1e23483669b1951f5de3e0b25f37df050d78eb0878233R219-R220) ```diff -
-
-
+{/each}
```
+
- ...
-
+
+ ...
| |
Refactor repetitive logic into a reactive Svelte statement for clarity and maintainability.___ **Refactor the symbol splitting logic into a reactive statement to clean up the templatecode and improve maintainability.** [interface/src/lib/components/market/candle/title.svelte [23]](https://github.com/Hu-Fi/Mr.Market/pull/150/files#diff-45a800628caa32fc13c0741ea3a9bed3ce24eba7e67d97b2f2e3a1de47dca0e6R23-R23) ```diff - {$CandlePair.symbol.split('/')[0]+"/"+$CandlePair.symbol.split('/')[1]} + + {formattedSymbol} ``` | |
| Enhancement |
Add error handling to navigation and interaction commands in tests.___ **Implement error handling for navigation and interaction commands to ensure the test cangracefully handle unexpected behaviors or errors during execution.** [interface/tests/grow/market-making.spec.ts [7-13]](https://github.com/Hu-Fi/Mr.Market/pull/150/files#diff-928b43985cc21ae40ff882f87b76d8a88f7161f9531de8faddf316d04dd54661R7-R13) ```diff -await page.goto('http://127.0.0.1:5173/grow'); -await page.getByTestId('market-making').click(); -await page.waitForURL('**/grow/market_making'); +try { + await page.goto('http://127.0.0.1:5173/grow'); + await page.getByTestId('market-making').click(); + await page.waitForURL('**/grow/market_making'); +} catch (error) { + console.error('Error navigating in market-making test:', error); +} ``` |
Simplify button styling by removing unnecessary hover effect.___ **Remove redundant hover style that matches the default background color, as it does notchange the button appearance on hover.** [interface/src/lib/components/grow/marketMaking/mmlist.svelte [21]](https://github.com/Hu-Fi/Mr.Market/pull/150/files#diff-1fed0bc48e85f6b717bcfc30555874426a7375199e7dbbaeafdcc55f10e61427R21-R21) ```diff - | |
| Best practice |
Use data-testid attributes instead of XPath for element selection in tests.___ **Replace direct XPath usage with a more robust method of selecting elements, such as usingdata-testid attributes, to improve test maintainability and readability.** [interface/tests/spot/candlestick.spec.ts [14]](https://github.com/Hu-Fi/Mr.Market/pull/150/files#diff-8a24368dc565d0a56029a364403befc5b6f509699945933ddca598469723358aR14-R14) ```diff -expect(await page.isVisible('//*[@id="candle_select_pair_modal"]/div/div[1]')).toBe(true) +expect(await page.getByTestId('candle_select_pair_modal').isVisible()).toBe(true) ``` |
Replace inline JavaScript in event handlers with named functions.___ **Avoid using inline JavaScript functions in theon:click attribute for better maintainability and testability. Instead, define a function in the Githubissues.
|
Type
Tests, Enhancement
Description
Changes walkthrough
8 files
coin.test.ts
Update Supported Exchanges in fetchOHLCV Testinterface/src/lib/helpers/hufi/coin.test.ts
'fetchOHLCV' test.
arbitrage.spec.ts
Add Arbitrage Creation Testsinterface/tests/grow/arbitrage.spec.ts
interactions and payment process.
market-making.spec.ts
Add Market Making Testsinterface/tests/grow/market-making.spec.ts - Added tests for market making feature including UI interactions.
home.spec.ts
Add Home Navigation Testsinterface/tests/home/home.spec.ts
navigation.
candlestick.spec.ts
Add Candlestick Pair Selector Testsinterface/tests/spot/candlestick.spec.ts
candlestick view.
navigation.spec.ts
Add Spot Navigation Testsinterface/tests/spot/navigation.spec.ts
section.
trading.spec.ts
Update Trading Testsinterface/tests/spot/trading.spec.ts
connect-wallet.spec.ts
Update Connect Wallet Testinterface/tests/wallet/connect-wallet.spec.ts - Updated the 'connect wallet' test to handle popup events.
1 files
README.md
Update README with Deployment InstructionsREADME.md
Railway.
2 files
en.json
Add New Time Interval to English Localizationinterface/src/i18n/en.json - Added new time interval '24hr' to English localization.
zh.json
Add New Time Interval to Chinese Localizationinterface/src/i18n/zh.json - Added new time interval '24hr' to Chinese localization.