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: ignore the copied entry_point.c #48297

Merged
merged 1 commit into from
Jun 6, 2023

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.

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

{
"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

@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
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

@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

Landed in d15652e

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

PR-URL: #48297
Reviewed-By: Tobias Nießen <[email protected]>
Reviewed-By: Gabriel Schulhof <[email protected]>
Reviewed-By: Joyee Cheung <[email protected]>
Reviewed-By: Darshan Sen <[email protected]>
Reviewed-By: LiviaMedeiros <[email protected]>
Reviewed-By: James M Snell <[email protected]>
@RafaelGSS RafaelGSS mentioned this pull request Jul 3, 2023
Ceres6 pushed a commit to Ceres6/node that referenced this pull request Aug 14, 2023
Add `test_cannot_run_js/entry_point.c` to
`test/js-native-api/.gitignore`.

PR-URL: nodejs#48297
Reviewed-By: Tobias Nießen <[email protected]>
Reviewed-By: Gabriel Schulhof <[email protected]>
Reviewed-By: Joyee Cheung <[email protected]>
Reviewed-By: Darshan Sen <[email protected]>
Reviewed-By: LiviaMedeiros <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Ceres6 pushed a commit to Ceres6/node that referenced this pull request Aug 14, 2023
Add `test_cannot_run_js/entry_point.c` to
`test/js-native-api/.gitignore`.

PR-URL: nodejs#48297
Reviewed-By: Tobias Nießen <[email protected]>
Reviewed-By: Gabriel Schulhof <[email protected]>
Reviewed-By: Joyee Cheung <[email protected]>
Reviewed-By: Darshan Sen <[email protected]>
Reviewed-By: LiviaMedeiros <[email protected]>
Reviewed-By: James M Snell <[email protected]>
ruyadorno pushed a commit that referenced this pull request Aug 29, 2023
Add `test_cannot_run_js/entry_point.c` to
`test/js-native-api/.gitignore`.

PR-URL: #48297
Reviewed-By: Tobias Nießen <[email protected]>
Reviewed-By: Gabriel Schulhof <[email protected]>
Reviewed-By: Joyee Cheung <[email protected]>
Reviewed-By: Darshan Sen <[email protected]>
Reviewed-By: LiviaMedeiros <[email protected]>
Reviewed-By: James M Snell <[email protected]>
@ruyadorno ruyadorno mentioned this pull request Aug 29, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
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
Development

Successfully merging this pull request may close these issues.

9 participants