Skip to content

test: ignore the copied entry_point.c#48297

Merged
nodejs-github-bot merged 1 commit intonodejs:mainfrom
lpinca:ignore/entry_point.c
Jun 6, 2023
Merged

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

Conversation

@lpinca
Copy link
Member

@lpinca lpinca commented Jun 2, 2023

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

@lpinca
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 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
Copy link
Member Author

lpinca commented Jun 2, 2023

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
Copy link
Member

targos commented Jun 2, 2023

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

@lpinca
Copy link
Member Author

lpinca commented Jun 2, 2023

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

cc: @gabrielschulhof

@gabrielschulhof
Copy link
Contributor

gabrielschulhof commented Jun 2, 2023 •
edited
Loading

@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 lpinca added the request-ci Add this label to start a Jenkins CI on a PR. label Jun 3, 2023
@github-actions 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
Copy link
Collaborator

nodejs-github-bot commented Jun 3, 2023

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

@nodejs-github-bot
Copy link
Collaborator

nodejs-github-bot commented Jun 3, 2023

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

@nodejs-github-bot
Copy link
Collaborator

nodejs-github-bot commented Jun 3, 2023

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

This was referenced Jun 4, 2023
Open
Open
@nodejs-github-bot
Copy link
Collaborator

nodejs-github-bot commented Jun 5, 2023

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

@github-actions github-actions bot mentioned this pull request Jun 6, 2023
Open
20 tasks
@nodejs-github-bot
Copy link
Collaborator

nodejs-github-bot commented Jun 6, 2023

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

@nodejs-github-bot
Copy link
Collaborator

nodejs-github-bot commented Jun 6, 2023

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

@lpinca lpinca added the commit-queue Add this label to land a pull request using GitHub Actions. label Jun 6, 2023
@nodejs-github-bot 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 nodejs-github-bot merged commit d15652e into nodejs:main Jun 6, 2023
@nodejs-github-bot
Copy link
Collaborator

nodejs-github-bot commented Jun 6, 2023

Landed in d15652e

@lpinca 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
@lpinca @RafaelGSS
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 RafaelGSS mentioned this pull request Jul 3, 2023
Merged
Ceres6 pushed a commit to Ceres6/node that referenced this pull request Aug 14, 2023
@lpinca @Ceres6
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
@lpinca @Ceres6
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 richardlau mentioned this pull request Aug 24, 2023
Closed
ruyadorno pushed a commit that referenced this pull request Aug 29, 2023
@lpinca @ruyadorno
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 ruyadorno mentioned this pull request Aug 29, 2023
Merged
@legendecas legendecas mentioned this pull request Sep 6, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jasnell jasnell jasnell approved these changes

@gabrielschulhof gabrielschulhof gabrielschulhof approved these changes

@tniessen tniessen tniessen approved these changes

@joyeecheung joyeecheung joyeecheung approved these changes

@RaisinTen RaisinTen RaisinTen approved these changes

@LiviaMedeiros 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

@lpinca @targos @gabrielschulhof @nodejs-github-bot @jasnell @tniessen @joyeecheung @RaisinTen @LiviaMedeiros