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

removes obsolete Node v14 from CI testing #8647

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

DougReeder
Copy link
Contributor

The tests pass under Node 18 and 20 on MacOS 13, but under Node 18 on Debian, there is one failure, so adding Node 18 and 20 to CI will have to wait for a future PR.

test.issue3179.js-local-http
#3179 conflicts synced, non-live replication (178ms)
1) #3179 conflicts synced, non-live sync

924 passing (46s)
17 pending
1 failing

  1. test.issue3179.js-local-http Removing conflicting revision doesn't sync from local to remote #3179 conflicts synced, non-live sync:
    TypeError: Cannot read properties of undefined (reading '0')
    at tests/integration/test.issue3179.js:85:54
    at process.processTicksAndRejections (node:internal/process/task_queues:95:5)

Copy link
Contributor

@lucidNTR lucidNTR left a comment

Choose a reason for hiding this comment

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

i see no downside to removing this, we should try to add an node 18 target too, are you sure this is reproducibly failing not just part of the flakyness? (see #8692)

@SourceR85
Copy link
Contributor

SourceR85 commented Jul 24, 2023

I expect, Node 18 should work fine.
In #8664, I've tinkered a little bit with the future, node 20 seems to be a little bit more problematic.

@DougReeder
Copy link
Contributor Author

If the tests are just flaky under Node 18 Debian, it's probably best to investigate before adding 18 to the CI. IMNSHO, tests failing in CI should mean something.

@SourceR85
Copy link
Contributor

@DougReeder the test-suite is flaky in every node version...
I don't expect some wonder, by just updating to another version 😉

Copy link
Member

@alxndrsn alxndrsn left a comment

Choose a reason for hiding this comment

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

Sadly this change breaks tests in new ways, e.g.

pouchdb-server (firefox, TYPE=find PLUGINS=pouchdb-find ADAPTERS=http npm test)

  1) test.ltgt.js $gt on nested field:

      expected 100 to equal 101
      + expected - actual

      -100
      +101

I think these should be addressed before merging.

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.

None yet

4 participants