FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

test: ignore the copied entry_point.c by lpinca · Pull Request #48297 · nodejs/node · GitHub

/ node Public

test: ignore the copied entry_point.c - #48297

Merged
nodejs-github-bot merged 1 commit into
nodejs:mainfrom
lpinca:ignore/entry_point.c
Jun 6, 2023
ConversationCommits (1)ChecksFiles changed
Merged

test: ignore the copied entry_point.c#48297
nodejs-github-bot merged 1 commit into
nodejs:mainfrom
lpinca:ignore/entry_point.c

Conversation

lpinca commented Jun 2, 2023

Copy link
Copy Markdown
Member

Add test_cannot_run_js/entry_point.c to
test/js-native-api/.gitignore.

test: ignore the copied entry_point.c
320ec77 
Add `test_cannot_run_js/entry_point.c` to
`test/js-native-api/.gitignore`.
nodejs-github-bot added needs-ci PRs that need a full CI run. node-api Issues and PRs related to the Node-API. test Issues and PRs related to the tests. labels Jun 2, 2023

lpinca commented Jun 2, 2023

Copy link
Copy Markdown
Member Author

See

node/test/js-native-api/test_cannot_run_js/binding.gyp

Lines 3 to 12 in 66f112d

{
"target_name": "copy_entry_point",
"type": "none",
"copies": [
{
"destination": ".",
"files": [ "../entry_point.c" ]
}
]
},
.

targos commented Jun 2, 2023

Copy link
Copy Markdown
Member

Do you know why it's copied instead of being referenced directly like in all other tests?

lpinca commented Jun 2, 2023

Copy link
Copy Markdown
Member Author

I don't know. The test was created like this.

cc: @gabrielschulhof

gabrielschulhof commented Jun 2, 2023 •
edited
Loading

Copy link
Copy Markdown
Contributor

@lpinca @targos somehow, having multiple targets in gyp that refer to a path above the root causes test failures. For example, when I tried to write the binding.gyp file as

{
  "target_defaults": {
    "sources": [
        "../entry_point.c",
    ]
  },
  "targets": [
    {
      "target_name": "test_cannot_run_js",
      "sources": [ "test_cannot_run_js.c" ],
      "defines": [ "NAPI_EXPERIMENTAL" ],
    },
    {
      "target_name": "test_pending_exception",
      "sources": [ "test_cannot_run_js.c" ],
      "defines": [ "NAPI_VERSION=8" ],
    }
  ]

I got this failure on https://ci.nodejs.org/job/node-test-commit-osx/52335/nodes=osx11-x64/console and a bunch of other platforms:

08:12:41 rm: ./Release/.deps/Release/obj.target/test_cannot_run_js/../entry_point.o.d.raw: No such file or directory

I tried referring to "../entry_point.c" from each target, and that failed too.

In the end, I had no choice but use this convoluted copy-the-file approach.

lpinca added the request-ci Add this label to start a Jenkins CI on a PR. label Jun 3, 2023
github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Jun 3, 2023

nodejs-github-bot commented Jun 3, 2023

Copy link
Copy Markdown
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/52062/

nodejs-github-bot commented Jun 3, 2023

Copy link
Copy Markdown
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/52063/

nodejs-github-bot commented Jun 3, 2023

Copy link
Copy Markdown
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/52065/

This was referenced Jun 4, 2023
Open
Open

nodejs-github-bot commented Jun 5, 2023

Copy link
Copy Markdown
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/52110/

github-actions Bot mentioned this pull request Jun 6, 2023
Open
20 tasks

nodejs-github-bot commented Jun 6, 2023

Copy link
Copy Markdown
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/52123/

nodejs-github-bot commented Jun 6, 2023

Copy link
Copy Markdown
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/52130/

lpinca added the commit-queue Add this label to land a pull request using GitHub Actions. label Jun 6, 2023
nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Jun 6, 2023
nodejs-github-bot merged commit d15652e into nodejs:main Jun 6, 2023

nodejs-github-bot commented Jun 6, 2023

Copy link
Copy Markdown
Collaborator

Landed in d15652e

lpinca deleted the ignore/entry_point.c branch June 6, 2023 16:23
This was referenced Jun 9, 2023
Open
Open
Open
Open
RafaelGSS pushed a commit that referenced this pull request Jul 3, 2023
test: ignore the copied entry_point.c
f1ead59 
Add `test_cannot_run_js/entry_point.c` to
`test/js-native-api/.gitignore`.

PR-URL: #48297
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Gabriel Schulhof <gabrielschulhof@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Darshan Sen <raisinten@gmail.com>
Reviewed-By: LiviaMedeiros <livia@cirno.name>
Reviewed-By: James M Snell <jasnell@gmail.com>
RafaelGSS mentioned this pull request Jul 3, 2023
Merged
Ceres6 pushed a commit to Ceres6/node that referenced this pull request Aug 14, 2023
test: ignore the copied entry_point.c
1c0af73 
Add `test_cannot_run_js/entry_point.c` to
`test/js-native-api/.gitignore`.

PR-URL: nodejs#48297
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Gabriel Schulhof <gabrielschulhof@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Darshan Sen <raisinten@gmail.com>
Reviewed-By: LiviaMedeiros <livia@cirno.name>
Reviewed-By: James M Snell <jasnell@gmail.com>
Ceres6 pushed a commit to Ceres6/node that referenced this pull request Aug 14, 2023
test: ignore the copied entry_point.c
03f94c1 
Add `test_cannot_run_js/entry_point.c` to
`test/js-native-api/.gitignore`.

PR-URL: nodejs#48297
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Gabriel Schulhof <gabrielschulhof@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Darshan Sen <raisinten@gmail.com>
Reviewed-By: LiviaMedeiros <livia@cirno.name>
Reviewed-By: James M Snell <jasnell@gmail.com>
richardlau mentioned this pull request Aug 24, 2023
Closed
ruyadorno pushed a commit that referenced this pull request Aug 29, 2023
test: ignore the copied entry_point.c
f35c4d3 
Add `test_cannot_run_js/entry_point.c` to
`test/js-native-api/.gitignore`.

PR-URL: #48297
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Gabriel Schulhof <gabrielschulhof@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Darshan Sen <raisinten@gmail.com>
Reviewed-By: LiviaMedeiros <livia@cirno.name>
Reviewed-By: James M Snell <jasnell@gmail.com>
ruyadorno mentioned this pull request Aug 29, 2023
Merged
legendecas mentioned this pull request Sep 6, 2023
Closed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

jasnell jasnell approved these changes
gabrielschulhof gabrielschulhof approved these changes
tniessen tniessen approved these changes
joyeecheung joyeecheung approved these changes
RaisinTen RaisinTen approved these changes
LiviaMedeiros LiviaMedeiros approved these changes

Assignees

No one assigned

Labels

needs-ci PRs that need a full CI run. node-api Issues and PRs related to the Node-API. test Issues and PRs related to the tests.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

9 participants

Back | FazBrowse Home | New Git URL