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

test: update compiled sqlite tests to match other tests #56446

Merged
merged 1 commit into from
Jan 5, 2025

Conversation

cjihrig
Copy link
Contributor

@cjihrig cjihrig commented Jan 3, 2025

This commit updates the sqlite compiled tests to be structured like other compiled tests.

Refs: #56347

This commit updates the sqlite compiled tests to be structured
like other compiled tests.

Refs: nodejs#56347
@nodejs-github-bot
Copy link
Collaborator

Review requested:

  • @nodejs/gyp

@nodejs-github-bot nodejs-github-bot added build Issues and PRs related to build files or the CI. needs-ci PRs that need a full CI run. labels Jan 3, 2025
Copy link

codecov bot commented Jan 3, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 88.52%. Comparing base (98d4ebc) to head (ac77830).
Report is 12 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main   #56446      +/-   ##
==========================================
- Coverage   88.53%   88.52%   -0.02%     
==========================================
  Files         657      657              
  Lines      190741   190741              
  Branches    36607    36606       -1     
==========================================
- Hits       168881   168855      -26     
- Misses      15036    15080      +44     
+ Partials     6824     6806      -18     

see 26 files with indirect coverage changes

Copy link
Member

@lpinca lpinca left a comment

Choose a reason for hiding this comment

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

RSLGTM

@lpinca lpinca added the request-ci Add this label to start a Jenkins CI on a PR. label Jan 3, 2025
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Jan 3, 2025
@nodejs-github-bot
Copy link
Collaborator

Copy link
Member

@sxa sxa left a comment

Choose a reason for hiding this comment

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

A shame that a "pessimistic rebuild" is needed, but I have confirmed that this fixes the original problem that was raised, so like @lpinca I will RSLGTM this one.

@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

nodejs-github-bot commented Jan 3, 2025

@cjihrig cjihrig added author ready PRs that have at least one approval, no pending requests for changes, and a CI started. commit-queue Add this label to land a pull request using GitHub Actions. and removed needs-ci PRs that need a full CI run. labels Jan 3, 2025
@aduh95 aduh95 added the needs-ci PRs that need a full CI run. label Jan 4, 2025
"target_name": "sqlite_extension",
"type": "shared_library",
"sources": [ "extension.c" ],
"include_dirs": [ "../../../deps/sqlite" ]
Copy link
Contributor

Choose a reason for hiding this comment

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

Shouldn't we be checking for --shared-sqlite here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not sure. We weren't before, but it's possible.

Copy link
Contributor

Choose a reason for hiding this comment

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

In any case, the test is skipped when using shared SQLite, so it's not a concern

@nodejs-github-bot nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Jan 5, 2025
@nodejs-github-bot nodejs-github-bot merged commit d0ff34f into nodejs:main Jan 5, 2025
78 checks passed
@nodejs-github-bot
Copy link
Collaborator

Landed in d0ff34f

@cjihrig cjihrig deleted the sqlite-tests branch January 5, 2025 15:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
author ready PRs that have at least one approval, no pending requests for changes, and a CI started. build Issues and PRs related to build files or the CI. needs-ci PRs that need a full CI run.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants