Skip to content

Conversation

miguelff
Copy link
Contributor

@miguelff miguelff commented Nov 6, 2023

Close https://github.com/prisma/team-orm/issues/517

Requires prisma/prisma-engines#4422 to be merged and integrated into prisma/prisma before merging

Copy link
Contributor

github-actions bot commented Nov 6, 2023

size-limit report 📦

Path Size
packages/client/runtime/library.d.ts 99.76 KB (0%)
packages/client/runtime/library.js 189.76 KB (0%)
packages/client/runtime/binary.js 616 KB (0%)
packages/client/runtime/edge.js 230.14 KB (0%)
packages/cli/build/index.js 5.11 MB (0%)

@miguelff miguelff self-assigned this Nov 6, 2023
@miguelff
Copy link
Contributor Author

miguelff commented Nov 6, 2023

Blocked until prisma/prisma-engines#4422 is merged

@miguelff miguelff marked this pull request as ready for review November 6, 2023 17:31
@miguelff miguelff requested a review from a team as a code owner November 6, 2023 17:31
@miguelff miguelff requested review from Druue and aqrln and removed request for a team November 6, 2023 17:31
@janpio janpio changed the title Unskip failing test in planetscale related to dates fix: Unskip failing test in planetscale related to dates Nov 6, 2023
@janpio janpio added this to the 5.6.0 milestone Nov 6, 2023
@janpio janpio self-requested a review November 7, 2023 16:01
@miguelff
Copy link
Contributor Author

miguelff commented Nov 7, 2023

Waiting to update prisma main with latest engines' after merging prisma/prisma-engines#4422, There's a PR open for the integration branch in #21831 and I'm looking for an (a priori unrelated) failing test

@miguelff
Copy link
Contributor Author

miguelff commented Nov 7, 2023

Actually the offending test in #21831 passes locally, so I will merge this PR as soon as the new version of engines is published and bumped either in main or here (whatever happens before)

❯ pe && cargo build -p query-engine-node-api && p && cd packages/client && PRISMA_QUERY_ENGINE_LIBRARY=/Users/miguel/GitHub/prisma/prisma-engines/target/debug/libquery_engine.dylib && pnpm run test:functional:code  --flavor=js_planetscale 15176
    Finished dev [unoptimized + debuginfo] target(s) in 0.64s

> @prisma/[email protected] test:functional:code /Users/miguel/GitHub/prisma/prisma/packages/client
> dotenv -e ../../.db.env -- node -r esbuild-register helpers/functional-test/run-tests.ts --no-types "--flavor=js_planetscale" "15176"

{
  column: 28,
  file: '/Users/miguel/GitHub/prisma/prisma/packages/engines/dist/index.js',
  length: 2,
  line: 16652,
  lineText: '          } else if (x2 === -0) {',
  namespace: '',
  suggestion: ''
}
Comparison with -0 using the "===" operator will also match 0
 PASS  tests/functional/issues/15176/tests.ts (5.547 s)
  issues.15176 (provider=sqlite)
    ○ skipped should update both updatedAt fields on a model
  issues.15176 (provider=sqlite, js_libsql)
    ○ skipped should update both updatedAt fields on a model
  issues.15176 (provider=postgresql)
    ○ skipped should update both updatedAt fields on a model
  issues.15176 (provider=postgresql, js_pg)
    ○ skipped should update both updatedAt fields on a model
  issues.15176 (provider=postgresql, js_neon)
    ○ skipped should update both updatedAt fields on a model
  issues.15176 (provider=mysql)
    ○ skipped should update both updatedAt fields on a model
  issues.15176 (provider=mysql, js_planetscale)
    ✓ should update both updatedAt fields on a model (97 ms)
  issues.15176 (provider=mongodb)
    ○ skipped should update both updatedAt fields on a model
  issues.15176 (provider=cockroachdb)
    ○ skipped should update both updatedAt fields on a model
  issues.15176 (provider=sqlserver)
    ○ skipped should update both updatedAt fields on a model

Test Suites: 1 passed, 1 total
Tests:       9 skipped, 1 passed, 10 total
Snapshots:   0 total
Time:        5.579 s
Ran all test suites matching /15176/i.

@janpio
Copy link
Contributor

janpio commented Nov 7, 2023

PR for engine update is here, but failing some tests that are blocking the merge: #21832 (I just rebased which will also run tests again, maybe it was just flaky)

@Jolg42
Copy link
Contributor

Jolg42 commented Nov 8, 2023

I actually merged the PR, but it was indeed not flaky. 😅
#21832 (comment)

See the failing test for Planetscale, it's the same error in this PR now that it's sync with main

@Jolg42
Copy link
Contributor

Jolg42 commented Nov 8, 2023

So technically not blocking this PR, but someone needs to have a look

@miguelff
Copy link
Contributor Author

miguelff commented Nov 8, 2023

I'm taking a look.

@miguelff
Copy link
Contributor Author

miguelff commented Nov 8, 2023

Ok, fix: prisma/prisma-engines#4426, The reason for the flakeyness, is that there is a lag between the create and the update, of up to a hundred milleseconds. Sometimes that makes it to the next second since epoch and the test passed. There was an underlying bug, tho. DateTimes didn't include franctionals in main.

I'm merging this, in the light of prisma/prisma-engines#4426

@miguelff miguelff merged commit 3cb0a0e into main Nov 8, 2023
@miguelff miguelff deleted the mff/517 branch November 8, 2023 11:31
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.

4 participants