🐛 fix: Align cache hashes #1248
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
matstyler
force-pushed
the
fix/align-cache-hashes
branch
from
9d0029e to
9f4ef40
Compare
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
matstyler
force-pushed
the
fix/align-cache-hashes
branch
from
3f1515b to
609f1a5
Compare
examples/metamask/package.json
Outdated
| @@ -16,6 +16,7 @@ | |||
| "dependencies": { | |||
| "@playwright/test": "1.48.2", | |||
| "@synthetixio/synpress": "workspace:*", | |||
| "@synthetixio/synpress-cache": "workspace:*", | |||
There was a problem hiding this comment.
you shouldn't need anything else other than synpress package in your tests
There was a problem hiding this comment.
If we use dependency of dependency, and both of them are internal packages, turbo doesn't refresh the dependencies properly.
There was a problem hiding this comment.
ok, but if I understand correctly, end user behavior will change and he will need two separate packages now, one for cache. it's breaking change which I would like to avoid.
There was a problem hiding this comment.
also, again - I think that end user should need main synpress package and nothing else. only additional wallets. we don't want to overcomplicate setup.
There was a problem hiding this comment.
no package behaviour change, the only problem is that we have examples in monorepo, not outside but I can remove this dependency now
examples/metamask/test/config.ts
Outdated
| @@ -0,0 +1,5 @@ | |||
| export default { | |||
| baseUrl: 'https://www.alphabot.app/', | |||
There was a problem hiding this comment.
the page user added as an example - this test case is more for you as a prove that this issue has been fixed
|
|
||
| await metamask.connectToDapp() | ||
|
|
||
| await page.waitForTimeout(2000) |
There was a problem hiding this comment.
we shouldnt need any waits, especially with timeout, they are flaky and bad practice
There was a problem hiding this comment.
it's just the same user example
There was a problem hiding this comment.
cool, but these are our official examples in our main repo - which are going to be copied over by other people. we don't want to have bad practices here.
There was a problem hiding this comment.
after review I will remove it, feel free to use it as a testing playground
|
|
||
| expect(createCacheForWalletSetupFunctionSpy).toHaveBeenCalledTimes(2) | ||
| expectCreateCacheForWalletSetupFunction(1, setupFunctions, 'hash1') | ||
| expectCreateCacheForWalletSetupFunction(2, setupFunctions, 'hash2') | ||
| }) | ||
|
|
||
| it('checks if cache already exists for each entry', async () => { | ||
| it.skip('checks if cache already exists for each entry', async () => { |
There was a problem hiding this comment.
most of tests are skipped, why?
There was a problem hiding this comment.
a leftover after development, should be reverted
| @@ -0,0 +1,12 @@ | |||
| export function extractWalletSetupFunction(sourceCode: string): string { | |||
| const match = sourceCode.match( | |||
| /defineWalletSetup\s*\([^,]*,\s*(async\s*\([^)]*\)\s*=>\s*{(?:[^{}]*|{(?:[^{}]*|{[^{}]*})*})*})\s*\)/ | |||
Check failure
Code scanning / CodeQL
Inefficient regular expression High
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix AI 20 days ago
To fix the problem, we need to modify the regular expression to remove the ambiguity and reduce the potential for excessive backtracking. This can be achieved by making the sub-expressions more specific and avoiding patterns that can match the same string in multiple ways. Specifically, we can replace [^{}]* with a more precise pattern that matches the intended input without allowing for excessive backtracking.
The best way to fix the problem is to rewrite the regular expression to avoid ambiguous sub-expressions and nested quantifiers. We can achieve this by using non-capturing groups and more specific character classes.
| @@ -2,3 +2,3 @@ | ||
| const match = sourceCode.match( | ||
| /defineWalletSetup\s*\([^,]*,\s*(async\s*\([^)]*\)\s*=>\s*{(?:[^{}]*|{(?:[^{}]*|{[^{}]*})*})*})\s*\)/ | ||
| /defineWalletSetup\s*\([^,]*,\s*(async\s*\([^)]*\)\s*=>\s*{(?:[^{}{}]*|{(?:[^{}{}]*|{[^{}{}]*})*})*})\s*\)/ | ||
| ) |
| @@ -0,0 +1,12 @@ | |||
| export function extractWalletSetupFunction(sourceCode: string): string { | |||
| const match = sourceCode.match( | |||
| /defineWalletSetup\s*\([^,]*,\s*(async\s*\([^)]*\)\s*=>\s*{(?:[^{}]*|{(?:[^{}]*|{[^{}]*})*})*})\s*\)/ | |||
Check failure
Code scanning / CodeQL
Inefficient regular expression High
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix AI 20 days ago
To fix the problem, we need to modify the regular expression to remove the ambiguity that leads to catastrophic backtracking. This can be achieved by making the pattern more specific and less ambiguous. In this case, we can replace the ambiguous [^{}]* with a more precise pattern that matches the expected structure of the input.
The best way to fix the problem without changing existing functionality is to rewrite the regular expression to avoid nested quantifiers and ambiguous patterns. Specifically, we can use a non-recursive approach to match the nested braces and their contents.
| @@ -2,3 +2,3 @@ | ||
| const match = sourceCode.match( | ||
| /defineWalletSetup\s*\([^,]*,\s*(async\s*\([^)]*\)\s*=>\s*{(?:[^{}]*|{(?:[^{}]*|{[^{}]*})*})*})\s*\)/ | ||
| /defineWalletSetup\s*\([^,]*,\s*(async\s*\([^)]*\)\s*=>\s*{[^{}]*(?:{[^{}]*}[^{}]*)*})\s*\)/ | ||
| ) |
No description provided.