Modularizing e2e tests #1898

VadimKovalenkoSNF · 2023-08-30T14:35:18Z

VadimKovalenkoSNF · 2023-08-30T14:38:07Z

Here is the first stage of applying multiple render API for tests. I realized that the best way to test multiple API is to add renderName option to CLI, which will allow to extend parameters for each e2e test. Moreover, we preserved this ability in the renderer builder when choosing specific option. I started with bm wiki, it’s a good example of wiki that has only WikimediaDesktop but not VisualEditor API. Each e2e test can sequentially apply the necessary renderer taken from renderName option. @kelson42 , let me know if this approach makes sense to you.

codecov · 2023-08-31T07:43:06Z

Codecov Report

All modified lines are covered by tests ✅

Comparison is base (d2768ef) 74.74% compared to head (6b51e74) 74.60%.
Report is 2 commits behind head on main.

❗ Current head 6b51e74 differs from pull request most recent head 3c5a0d0. Consider uploading reports for the commit 3c5a0d0 to get more accurate results

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1898      +/-   ##
==========================================
- Coverage   74.74%   74.60%   -0.14%     
==========================================
  Files          33       33              
  Lines        2811     2812       +1     
  Branches      626      626              
==========================================
- Hits         2101     2098       -3     
- Misses        613      618       +5     
+ Partials       97       96       -1

Files	Coverage Δ
src/mwoffliner.lib.ts	`74.81% <100.00%> (+0.09%)`	⬆️
src/util/const.ts	`100.00% <100.00%> (ø)`

... and 4 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

kelson42

I have left a few comments

test/e2e/rendererList.test.ts

src/parameterList.ts

test/e2e/rendererList.ts

test/e2e/rendererList.test.ts

test/e2e/bm.e2e.test.ts

kelson42 · 2023-09-06T06:17:30Z

@VadimKovalenkoSNF Can you please:

Fix a few comments regarding the code allowing to force a dedicated render
Move this part in an other PR (wihtout the testing part)

That way, I would merge straight, as this LGTM. We could then keep this PR for focus on the testing only.

kelson42 · 2023-09-08T12:19:42Z

@VadimKovalenkoSNF This PR should be rebased/revamped, now that I have merged #1901. Hopefully, then I will better understand the way how you solve the problem.

VadimKovalenkoSNF · 2023-09-20T15:59:34Z

test/e2e/treatments.e2e.test.ts

+  const outFiles = await mwoffliner.execute(parameters)
+  await expect(zimcheck(outFiles[0].outFile)).resolves.not.toThrowError()
+
+  const articleFromDump = await zimdump(`show --url A/${parameters.articleList} ${outFiles[0].outFile}`)


There is the ability to get an article HTML form dump and test it directly along with other dump output. This might be useful for e2e testing.

I don't really understand why yor comment come here, but zimcheck should be run everywhere, so why I have eachtime to request it? Create a test template/fonction/classs so this is part is present only once in the whole codebase.

kelson42

Once we are at the end of the simplification, we should fix the nodet test as well.

test/e2e/bm.e2e.test.ts

test/e2e/rendererList.test.ts

kelson42 · 2023-09-20T18:15:12Z

test/e2e/treatments.e2e.test.ts

+describe('Treatments e2e', () => {
+  for (const renderer of renderers) {
+    if (renderer === 'WikimediaDesktop') {
+      test('WikimediaDesktop e2e', async () => {
+        await commonTreatmentTest(renderer)
+      })
+    }
+    if (renderer === 'VisualEditor') {
+      test('VisualEditor e2e', async () => {
+        await commonTreatmentTest(renderer)
+      })
+    }
+  }
+})


You can do better: should be doable in 3 lines.

Done, pls check.

Better, but why we have an if on Render names? All renders should be tested.

I made it just for safety for cases if some wiki doesn't support a specific wiki.

This test file has been refactored and renamed to test/e2e/en.e2e.test.ts.

test/e2e/treatments.e2e.test.ts

kelson42 · 2023-09-20T18:17:50Z

test/e2e/treatments.e2e.test.ts

+jest.setTimeout(200000)
+
+let zimcheckIsAvailable
+let zimdumpIsAvailable
+
+beforeAll(async () => {
+  zimcheckIsAvailable = await zimcheckAvailable()
+  zimdumpIsAvailable = await zimdumpAvailable()
+})
+


If we could avoid to do that for each test, this would be really better. No way to do simpler?

This beforeAll hook is placed outside describe blocks, so it will execute only once.

The problem is not the execution time, but that we have to place this piece of code almost in all e2e test.

It's not an execution problem, it's a code problem, these lines should be only once coded in the whole codebase.

Done in test/testAllRenders.ts

test/e2e/treatments.e2e.test.ts

VadimKovalenkoSNF · 2023-09-22T07:16:58Z

Once we are at the end of the simplification, we should fix the nodet test as well.

I fixed this in the scope of this PR.

VadimKovalenkoSNF · 2023-09-22T07:25:04Z

There is a side effect of e2e testing of all renderers is drastically increased time of test running.

kelson42 · 2023-09-26T05:58:08Z

There is a side effect of e2e testing of all renderers is drastically increased time of test running.

Yes, this will be a problem and we will have to handle it at some point (but not now) to have e2e which are smaller/quicker to execute, so no scrape with many articles

kelson42

Still too complex, we should have one test with no boilerplate, nothing which is repeated from test to test

kelson42

A lot better, but still a few things to fix

test/testAllRenders.ts

test/unit/saveArticles.test.ts

kelson42

Test zimcheck/zimdump only once in the whole test process!
rebase

…le renderers

VadimKovalenkoSNF requested a review from kelson42 August 30, 2023 14:38

VadimKovalenkoSNF force-pushed the 1887-e2e-modularized branch from 4420c39 to ee395ed Compare August 31, 2023 07:28

kelson42 requested changes Sep 5, 2023

View reviewed changes

VadimKovalenkoSNF force-pushed the 1887-e2e-modularized branch from ee395ed to 750861e Compare September 20, 2023 15:54

VadimKovalenkoSNF commented Sep 20, 2023

View reviewed changes

VadimKovalenkoSNF marked this pull request as ready for review September 20, 2023 16:12

VadimKovalenkoSNF requested a review from kelson42 September 20, 2023 16:12

kelson42 force-pushed the 1887-e2e-modularized branch from 750861e to 5140a17 Compare September 20, 2023 17:55

kelson42 requested changes Sep 20, 2023

View reviewed changes

VadimKovalenkoSNF force-pushed the 1887-e2e-modularized branch from 5140a17 to 48b6c40 Compare September 21, 2023 09:10

VadimKovalenkoSNF requested a review from kelson42 September 22, 2023 07:17

VadimKovalenkoSNF mentioned this pull request Sep 22, 2023

Implement new Wikimedia mobile end-point support/render #1903

Merged

kelson42 requested changes Sep 28, 2023

View reviewed changes

VadimKovalenkoSNF requested a review from kelson42 September 29, 2023 17:09

kelson42 requested changes Sep 30, 2023

View reviewed changes

test/testAllRenders.ts Outdated Show resolved Hide resolved

test/testAllRenders.ts Outdated Show resolved Hide resolved

test/unit/saveArticles.test.ts Outdated Show resolved Hide resolved

VadimKovalenkoSNF requested a review from kelson42 October 3, 2023 07:46

kelson42 requested changes Oct 4, 2023

View reviewed changes

VadimKovalenkoSNF added 6 commits October 4, 2023 09:42

Introduce renderName parameter for scrapping proccess

c477efd

Update renderList test

2befc40

Add e2e test for article treatments per renderer

d8c6122

Minor refactoring of treatments e2e tests

2962723

Optimize nodet tests

47b408f

Connect VE to bm.e2e.test.ts

a98d780

VadimKovalenkoSNF added 8 commits October 4, 2023 09:43

Increase timeout to bm.e2e.test.ts since it scrapes data using multip…

9afab57

…le renderers

Add multiple variants of e2e templates

003dfd2

Update e2e template

956fd7a

Improve e2e template

0e3e103

Roll back bm.e2e.test.ts

4f83356

Remove redundant rendererList test

7d9ff2f

Exit test process if no zim tools available

d84b294

Refactor zim tool checking as lazy method

3c5a0d0

VadimKovalenkoSNF force-pushed the 1887-e2e-modularized branch from f35066e to 3c5a0d0 Compare October 4, 2023 06:44

VadimKovalenkoSNF requested a review from kelson42 October 4, 2023 06:46

kelson42 merged commit af4f200 into main Oct 5, 2023
1 check passed

kelson42 deleted the 1887-e2e-modularized branch October 5, 2023 04:46

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Modularizing e2e tests #1898

Modularizing e2e tests #1898

VadimKovalenkoSNF commented Aug 30, 2023

VadimKovalenkoSNF commented Aug 30, 2023

codecov bot commented Aug 31, 2023 •

edited

Loading

kelson42 left a comment

kelson42 commented Sep 6, 2023

kelson42 commented Sep 8, 2023

VadimKovalenkoSNF Sep 20, 2023

kelson42 Sep 26, 2023

kelson42 left a comment

kelson42 Sep 20, 2023

VadimKovalenkoSNF Sep 22, 2023

kelson42 Sep 26, 2023

VadimKovalenkoSNF Sep 29, 2023

VadimKovalenkoSNF Sep 29, 2023

kelson42 Sep 20, 2023

VadimKovalenkoSNF Sep 21, 2023

kelson42 Sep 26, 2023

kelson42 Sep 26, 2023

VadimKovalenkoSNF Sep 29, 2023

VadimKovalenkoSNF commented Sep 22, 2023 •

edited

Loading

VadimKovalenkoSNF commented Sep 22, 2023

kelson42 commented Sep 26, 2023 •

edited

Loading

kelson42 left a comment

kelson42 left a comment

kelson42 left a comment •

edited

Loading

Modularizing e2e tests #1898

Modularizing e2e tests #1898

Conversation

VadimKovalenkoSNF commented Aug 30, 2023

VadimKovalenkoSNF commented Aug 30, 2023

codecov bot commented Aug 31, 2023 • edited Loading

Codecov Report

kelson42 left a comment

Choose a reason for hiding this comment

kelson42 commented Sep 6, 2023

kelson42 commented Sep 8, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kelson42 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

VadimKovalenkoSNF commented Sep 22, 2023 • edited Loading

VadimKovalenkoSNF commented Sep 22, 2023

kelson42 commented Sep 26, 2023 • edited Loading

kelson42 left a comment

Choose a reason for hiding this comment

kelson42 left a comment

Choose a reason for hiding this comment

kelson42 left a comment • edited Loading

Choose a reason for hiding this comment

codecov bot commented Aug 31, 2023 •

edited

Loading

VadimKovalenkoSNF commented Sep 22, 2023 •

edited

Loading

kelson42 commented Sep 26, 2023 •

edited

Loading

kelson42 left a comment •

edited

Loading