Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix a few geo tests in the CI #2108

Merged
merged 1 commit into from
Dec 4, 2024
Merged

Fix a few geo tests in the CI #2108

merged 1 commit into from
Dec 4, 2024

Conversation

kelson42
Copy link
Collaborator

@kelson42 kelson42 commented Dec 3, 2024

Values have changed upstream

@kelson42 kelson42 requested a review from audiodude December 3, 2024 18:43
@kelson42 kelson42 added this to the 1.14.0 milestone Dec 3, 2024
Copy link
Member

@audiodude audiodude left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CI still failing...

@kelson42
Copy link
Collaborator Author

kelson42 commented Dec 3, 2024

@audiodude Yes, this is because we have lost WP1 selection files, see openzim/wp1#171 (comment). I propose to fix this by creating an arbitrary selection in WP1 to fix the CI for the time beeing.

@kelson42 kelson42 marked this pull request as draft December 3, 2024 19:57
@audiodude
Copy link
Member

@audiodude Yes, this is because we have lost WP1 selection files, see openzim/wp1#171 (comment). I propose to fix this by creating an arbitrary selection in WP1 to fix the CI for the time beeing.

Is that really the issue? Looking at the failed test run () I see this:

FAIL test/e2e/en10.e2e.test.ts
  ● Test suite failed to run

    Your test suite must contain at least one test.

      at onResult (node_modules/@jest/core/build/TestScheduler.js:133:18)
      at node_modules/@jest/core/build/TestScheduler.js:254:19
      at node_modules/emittery/index.js:363:13
          at Array.map (<anonymous>)
      at Emittery.emit (node_modules/emittery/index.js:361:23)

Though I don't understand how that wasn't a problem in the past.

@audiodude
Copy link
Member

Oh I see it now:

  console.error
    [error] [2024-12-04T03:32:25.313Z] Failed to read articleList from URL: https://download.openzim.org/wp1/enwiki/tops/10.tsv

The message is incorrect, because the tests only get defined after the testAllRenders method executes. Also it's hard to read the CI output because of #2038

@audiodude audiodude marked this pull request as ready for review December 4, 2024 03:59
@audiodude audiodude merged commit a3169e5 into main Dec 4, 2024
3 of 4 checks passed
@audiodude audiodude deleted the fix-geo-ci branch December 4, 2024 03:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants