stepan662
commented
4 months ago I like this approach @wfjsw . However I will need to check this in detail. Could you please fix the eslint and typescript errors?
Closed wfjsw closed 3 months ago
stepan662
commented
4 months ago I like this approach @wfjsw . However I will need to check this in detail. Could you please fix the eslint and typescript errors?
stepan662
commented
4 months ago The Prepare packages pipeline won't run from PR, but the other pipelines should be passing.
stepan662
commented
4 months ago I'm afraid it doesn't always detect the translations and thats why the tests are failing. Would you be able to go through the tests and look at it? You can run them locally with pnpm build + pnpm test in root folder.
This is quite a delicate change, we need to be sure that it will work reliably. If you'd feel it is too much work for you, I can do it once I have more time, I think the direction is right, I didn't like the xpath solution either π.
wfjsw
commented
4 months ago Real world test also shows that it will miss 1 or 2 elements. Not totally sure what's happening though. Will try to look into it.
wfjsw
commented
4 months ago I think I got it. Fixed.
wfjsw
commented
4 months ago Still failing on this. Honestly I didn't find how the original code handle it.
@tolgee/web:test: FAIL src/__test__/observer.test.ts (30.353 s)
@tolgee/web:test: β invisble observer βΊ observer βΊ key only attribute
@tolgee/web:test:
@tolgee/web:test: expect(received).toBeFalsy()
@tolgee/web:test:
@tolgee/web:test: Received: "true"
@tolgee/web:test:
@tolgee/web:test: Ignored nodes: comments, script, style
@tolgee/web:test: <html>
@tolgee/web:test: <head />
@tolgee/web:test: <body>
@tolgee/web:test:
@tolgee/web:test:
@tolgee/web:test: <span
@tolgee/web:test: _tolgee="true"
@tolgee/web:test: data-testid="translation"
@tolgee/web:test: >
@tolgee/web:test: Hello
@tolgee/web:test: </span>
@tolgee/web:test:
@tolgee/web:test:
@tolgee/web:test: </body>
@tolgee/web:test: </html>
@tolgee/web:test:
@tolgee/web:test: 105 | .queryByTestId('translation')
@tolgee/web:test: 106 | ?.getAttribute(TOLGEE_ATTRIBUTE_NAME)
@tolgee/web:test: > 107 | ).toBeFalsy();
@tolgee/web:test: | ^
@tolgee/web:test: 108 | });
@tolgee/web:test: 109 | });
@tolgee/web:test: 110 | });
@tolgee/web:test:
@tolgee/web:test: at src/__test__/testObserver.ts:107:11
@tolgee/web:test: at runWithExpensiveErrorDiagnosticsDisabled (../../node_modules/.pnpm/@testing-library+dom@8.20.1/node_modules/@testing-library/dom/dist/config.js:47:12)
@tolgee/web:test: at checkCallback (../../node_modules/.pnpm/@testing-library+dom@8.20.1/node_modules/@testing-library/dom/dist/wait-for.js:127:77)
@tolgee/web:test: at checkRealTimersCallback (../../node_modules/.pnpm/@testing-library+dom@8.20.1/node_modules/@testing-library/dom/dist/wait-for.js:121:16)
@tolgee/web:test: at Timeout.task [as _onTimeout] (../../node_modules/.pnpm/jsdom@16.7.0/node_modules/jsdom/lib/jsdom/browser/Window.js:516:19)Everything seems to be fixed now
wfjsw
commented
4 months ago The package.json is giving me an obsolete pnpm version that cannot understand the lock file in the repo and creates quite a few issues.
JanCizmar
commented
4 months ago @stepan662 might know...
stepan662
commented
4 months ago @wfjsw are you sure you are using latest pnpm, I've tested it and it works fine with v8.14.1
wfjsw
commented
4 months ago corepack enable reads packageManager in package.json and gave me pnpm 7.9.4.
stepan662
commented
4 months ago Ah, ok. I'm still on node 20, and it doesn't check that I guess. But I think if you upgrade that field to 8.* it should work, we are using it anyway.
stepan662
commented
4 months ago Ah, sorry I was not familiar with corepack tool, looks cool. I didn't update the packageManager field, but in pipelines and everywhere we're using pnpm 8.6.12. Feel free to update the that.
stepan662
commented
4 months ago I've tested it on our platform and also I didn't find any issue, so I think we can merge it.
stepan662
commented
4 months ago I've tested it with e2e tests and it is failing (https://github.com/tolgee/tolgee-js/pull/3297). We have an utility for running e2e tests locally, it's used in the test pipeline (https://github.com/tolgee/tolgee-js/blob/main/.github/workflows/test.yml#L192).
wfjsw
commented
4 months ago The issue above is fixed, however my local E2E suite seems a bit off:
23:06:37 : Running: dev.cy.ts (1 of 2)
23:06:39 [app]: tolgee_js_e2e_server | Hibernate: select bj1_0.id,bj1_0.author_id,bj1_0.chunk_size,bj1_0.created_at,bj1_0.debounce_duration_in_ms,bj1_0.debounce_max_wait_time_in_ms,bj1_0.debouncing_key,bj1_0.hidden,bj1_0.job_character,bj1_0.last_debouncing_event,bj1_0.max_per_job_concurrency,bj1_0.params,bj1_0.project_id,bj1_0.status,bj1_0.target,bj1_0.total_chunks,bj1_0.total_items,bj1_0.type,bj1_0.updated_at from tolgee_batch_job bj1_0 where 1=0 and bj1_0.status in (?,?,?,?) and bj1_0.updated_at<?
23:06:39 [app]: tolgee_js_e2e_server | Hibernate: select bj1_0.id from tolgee_batch_job bj1_0 join tolgee_batch_job_chunk_execution bjce1_0 on bjce1_0.batch_job_id=bj1_0.id where 1=0 group by bj1_0.id having max(bjce1_0.updated_at)<?
23:06:40 :
23:06:40 :
23:06:40 : Vue app in dev mode
23:06:40 : standard example app test
23:06:42 : β Has title (1024ms)
23:06:42 : β Has default items (37ms)
23:06:42 : β Has buttons (23ms)
23:06:42 : β Deletes item (113ms)
23:06:43 : β Adds item (361ms)
23:06:43 : β is translated to german (184ms)
23:06:43 : translation methods test
23:06:43 : for language en (run 1)
23:06:43 : β contains "This is default" 2 times (234ms)
23:06:43 : β contains "This is a key" 5 times (19ms)
23:06:43 : β contains "This is key with params value value2" 3 times (24ms)
23:06:43 : for language de (run 1)
23:06:43 : β contains "This is default" 2 times (205ms)
23:06:43 : β contains "Dies ist ein SchlΓΌssel" 5 times (17ms)
23:06:43 : β contains "Dies ist ein SchlΓΌssel mit den Parametern value value2" 3 times (17ms)
23:06:43 : for language en (run 2)
23:06:44 : β contains "This is default" 2 times (187ms)
23:06:44 : β contains "This is a key" 5 times (19ms)
23:06:44 : β contains "This is key with params value value2" 3 times (16ms)
23:06:44 : for language de (run 2)
23:06:44 : β contains "This is default" 2 times (186ms)
23:06:44 : β contains "Dies ist ein SchlΓΌssel" 5 times (18ms)
23:06:44 : β contains "Dies ist ein SchlΓΌssel mit den Parametern value value2" 3 times (18ms)
23:06:44 : translation methods test
23:06:44 : for language en
23:06:45 : β contains "This is a key in namespace" 2 times (665ms)
23:06:45 : β contains "This is a key" 1 times (16ms)
23:06:45 : for language cs
23:06:45 : β contains "Toto je klΓΔ v namespace" 2 times (191ms)
23:06:45 : β contains "Toto je klΓΔ" 1 times (23ms)
23:06:45 : standard example app dev test
23:06:49 [app]: tolgee_js_e2e_server | Hibernate: select bj1_0.id,bj1_0.author_id,bj1_0.chunk_size,bj1_0.created_at,bj1_0.debounce_duration_in_ms,bj1_0.debounce_max_wait_time_in_ms,bj1_0.debouncing_key,bj1_0.hidden,bj1_0.job_character,bj1_0.last_debouncing_event,bj1_0.max_per_job_concurrency,bj1_0.params,bj1_0.project_id,bj1_0.status,bj1_0.target,bj1_0.total_chunks,bj1_0.total_items,bj1_0.type,bj1_0.updated_at from tolgee_batch_job bj1_0 where 1=0 and bj1_0.status in (?,?,?,?) and bj1_0.updated_at<?
23:06:49 [app]: tolgee_js_e2e_server | Hibernate: select bj1_0.id from tolgee_batch_job bj1_0 join tolgee_batch_job_chunk_execution bjce1_0 on bjce1_0.batch_job_id=bj1_0.id where 1=0 group by bj1_0.id having max(bjce1_0.updated_at)<?
23:06:49 : 1) Shows loading on visit
23:06:53 : 2) title can be translated
23:06:58 : 3) placeholder can be translated
23:06:58 :
23:06:58 :
23:06:58 : 22 passing (18s)
23:06:58 : 3 failing
23:06:58 : 1) Vue app in dev mode
23:06:58 : standard example app dev test
23:06:58 : Shows loading on visit:
23:06:58 : AssertionError: Timed out retrying after 4000ms: Expected to find content: 'Loading...' but never did.
23:06:58 : at Context.eval (webpack:///./cypress/common/exampleAppDevTest.ts:15:34)
23:06:58 : 2) Vue app in dev mode
23:06:58 : standard example app dev test
23:06:58 : title can be translated:
23:06:58 : AssertionError: Timed out retrying after 4000ms: expected undefined to exist
23:06:58 : at Context.eval (webpack:///./cypress/common/exampleAppDevTest.ts:21:60)
23:06:58 : 3) Vue app in dev mode
23:06:58 : standard example app dev test
23:06:58 : placeholder can be translated:
23:06:58 : AssertionError: Timed out retrying after 4000ms: expected undefined to exist
23:06:58 : at Context.eval (webpack:///./cypress/common/exampleAppDevTest.ts:29:60)
23:06:59 :
23:06:59 : (Results)
23:06:59 :
23:06:59 : ββββββββββββββββββββββββββββββββββββββββββββββββββββββββββββββββββββββββββββββββββββββββββββββββββ
23:06:59 : β Tests: 25 β
23:06:59 : β Passing: 22 β
23:06:59 : β Failing: 3 β
23:06:59 : β Pending: 0 β
23:06:59 : β Skipped: 0 β
23:06:59 : β Screenshots: 3 β
23:06:59 : β Video: true β
23:06:59 : β Duration: 17 seconds β
23:06:59 : β Spec Ran: dev.cy.ts β
23:06:59 : ββββββββββββββββββββββββββββββββββββββββββββββββββββββββββββββββββββββββββββββββββββββββββββββββββThe E2e tests can be quite sensitive. Not sure what's going on there. Howerver I've ran all the test in the pipeline above and it seems that there is not a problem with vue, but ngx. It's failing in both unit tests and e2e tests. Other than that it seems all fine.
stepan662
commented
3 months ago Seems like everything is working properly, so we can merge this. Thanks a lot @wfjsw and sorry for lengthy process, this is a really cool improvement π
meta.nodesclean up based onmutation.removedNodes(opt chance: theremovedNodeswalker may want a better filter`)