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.

Sign up for GitHub

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

Jump to bottom

GH-2344: Yarn fails on windows CI-BUILD #2410

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

GH-2344: Yarn fails on windows CI-BUILD #2410

wants to merge 1 commit into from

Conversation

OyvindLGjesdal
Copy link
Contributor

@OyvindLGjesdal OyvindLGjesdal commented Apr 10, 2024 •
edited

GitHub issue resolved #2344

Pull request Description:

Looked at the links provided by @kinow in #2344 (comment) and tried to change ' with \".

This gave a new error which looks like crossenv doesn't support := on windows, in ${FUSEKI_PORT:=3030} (and PORT).
Looked at crossenv on github and I think it is not available.

I looked at maybe changing the shell to run from in windows, by adding a .npmrc file, but then I got license (RAT) errors when running the CI, and instead of adding it to exclude list, I asked GPT figured the operator is probably not available in powershell. I ended up using the ENV vars only since FUSEKI_PORT is already used without fallback values in other locations of the package.json file.

Glanced at the final e2e output in https://github.com/OyvindLGjesdal/jena/actions/runs/8623049304 and it looks good to my untrained eyes.

Have left to check if it breaks anything for Linux and Mac, and will create a single commit.

Output when only fixing the apos -> escaped quote:

[INFO] $ cross-env FUSEKI_PORT="${FUSEKI_PORT:=3030}" PORT="${PORT:=8080}" concurrently --names 'SERVER,CLIENT,TESTS' --prefix-colors 'yellow,blue,green' --success 'first' --kill-others "yarn run serve:fuseki" "yarn wait-on http://localhost:${FUSEKI_PORT}/$/ping && yarn run dev" "yarn wait-on http-get://localhost:${PORT}/index.html && cypress run $@"
[INFO] [TESTS] $ C:\jena\jena-fuseki2\jena-fuseki-ui\node_modules\.bin\wait-on http-get://localhost:${PORT:=8080}/index.html
[INFO] [CLIENT] $ C:\jena\jena-fuseki2\jena-fuseki-ui\node_modules\.bin\wait-on http://localhost:${FUSEKI_PORT:=3030}/$/ping
[INFO] [SERVER] $ nodemon src/services/mock/json-server.js
[INFO] [SERVER] [nodemon] 3.1.0
[INFO] [SERVER] [nodemon] to restart at any time, enter `rs`
[INFO] [SERVER] [nodemon] watching path(s): *.*
[INFO] [SERVER] [nodemon] watching extensions: js,mjs,cjs,json
[INFO] [SERVER] [nodemon] starting `node src/services/mock/json-server.js`
[INFO] [SERVER] node:events:496
[INFO] [SERVER]       throw er; // Unhandled 'error' event
[INFO] [SERVER]       ^
[INFO] [SERVER] 
[INFO] [SERVER] Error: listen EACCES: permission denied ${FUSEKI_PORT:=3030}
[INFO] [SERVER]     at Server.setupListenHandle [as _listen2] (node:net:1855:21)
[INFO] [SERVER]     at listenInCluster (node:net:1920:12)
[INFO] [SERVER]     at Server.listen (node:net:2025:5)
[INFO] [SERVER]     at Function.listen (C:\jena\jena-fuseki2\jena-fuseki-ui\node_modules\express\lib\application.js:635:24)
[INFO] [SERVER]     at Object.<anonymous> (C:\jena\jena-fuseki2\jena-fuseki-ui\src\services\mock\json-server.js:294:8)
[INFO] [SERVER]     at Module._compile (node:internal/modules/cjs/loader:1376:14)
[INFO] [SERVER]     at Module._extensions..js (node:internal/modules/cjs/loader:1435:10)
[INFO] [SERVER]     at Module.load (node:internal/modules/cjs/loader:1207:32)
[INFO] [SERVER]     at Module._load (node:internal/modules/cjs/loader:1023:12)
[INFO] [SERVER]     at Function.executeUserEntryPoint [as runMain] (node:internal/modules/run_main:135:12)
[INFO] [SERVER] Emitted 'error' event on Server instance at:
[INFO] [SERVER]     at emitErrorNT (node:net:1899:8)
[INFO] [SERVER]     at process.processTicksAndRejections (node:internal/process/task_queues:82:21) {
[INFO] [SERVER]   code: 'EACCES',
[INFO] [SERVER]   errno: -4092,
[INFO] [SERVER]   syscall: 'listen',
[INFO] [SERVER]   address: '${FUSEKI_PORT:=3030}',
[INFO] [SERVER]   port: -1
[INFO] [SERVER] }
[INFO] [SERVER] 
[INFO] [SERVER] Node.js v20.11.0
[INFO] [SERVER] [nodemon] app crashed - waiting for file changes before starting...
  • Tests are included.
  • Documentation change and updates are provided for the Apache Jena website
  • Commits have been squashed to remove intermediate development commit messages.
  • Key commit messages start with the issue number (GH-xxxx)

By submitting this pull request, I acknowledge that I am making a contribution to the Apache Software Foundation under the terms and conditions of the Contributor's Agreement.

See the Apache Jena "Contributing" guide.

@OyvindLGjesdal
Copy link
Contributor Author

Tested and ran on linux job which looks good, but confused me a bit first because the ordering of running the tests changed between runs. I guess the removal of the default operator may mean that the e2e becomes more clunky to run locally or standalone? FUSEKI_PORTS and PORTS changed between each run.

There are a few more errors after the tests for windows:

[INFO] --> Sending SIGTERM to other processes..
[INFO] [CLIENT] yarn wait-on http://localhost:%FUSEKI_PORT%/$/ping && yarn run dev exited with code 1
[INFO] --> Sending SIGTERM to other processes..
[INFO] [SERVER] yarn run serve:fuseki exited with code 1
[INFO] Done in 39.23s.

afs
afs previously approved these changes Apr 15, 2024
View reviewed changes
Copy link
Member

@afs afs left a comment

Choose a reason for hiding this comment

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

This looks like an improvement.

I ran before and after Windows jobs (GH actions).
I see http://localhost:%FUSEKI_PORT%/$/ping in the breaking "before" which might be due to messing up the substitution and/or quoting and a lot of stacktraces.

With this PR, the Windows build does not print that nor have stacktraces.

There are a few more errors after the tests for windows:

(also untrained eyes 🙈 ) Are they errors or is that the test runner clearing up?
With this PR, I see: "All specs passed" which didn't happen before.

[INFO] [TESTS]     ✔  All specs passed!                        00:19       13       13        -        -        -  
[INFO] [TESTS] 
[INFO] [TESTS] yarn wait-on http-get://localhost:%PORT%/index.html && cypress run $@ exited with code 0
[INFO] --> Sending SIGTERM to other processes..
[INFO] [SERVER] yarn run serve:fuseki exited with code 1
[INFO] --> Sending SIGTERM to other processes..
[INFO] [CLIENT] yarn wait-on http://localhost:%FUSEKI_PORT%/$/ping && yarn run dev exited with code 1
[INFO] Done in 37.36s.

@afs afs requested a review from kinow April 15, 2024 08:36
kinow
kinow reviewed Apr 15, 2024
View reviewed changes
Copy link
Member

@kinow kinow left a comment

Choose a reason for hiding this comment

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

I think this broke the e2e tests on Linux.

kinow@ranma:~/Development/java/jena/jena/jena-fuseki2/jena-fuseki-ui$ yarn test:e2e
yarn run v1.22.19
$ cross-env FUSEKI_PORT="${FUSEKI_PORT}" PORT="${PORT}" concurrently --names 'SERVER,CLIENT,TESTS' --prefix-colors 'yellow,blue,green' --success 'first' --kill-others "yarn run serve:fuseki" "yarn wait-on http://localhost:${FUSEKI_PORT}/$/ping && yarn run dev" "yarn wait-on http-get://localhost:${PORT}/index.html && cypress run $@"
$ nodemon src/services/mock/json-server.js
$ /home/kinow/Development/java/jena/jena/jena-fuseki2/jena-fuseki-ui/node_modules/.bin/wait-on 'http://localhost:/$/ping'
$ /home/kinow/Development/java/jena/jena/jena-fuseki2/jena-fuseki-ui/node_modules/.bin/wait-on http-get://localhost:/index.html
[SERVER] [nodemon] 3.1.0
[SERVER] [nodemon] to restart at any time, enter `rs`
[SERVER] [nodemon] watching path(s): *.*
[SERVER] [nodemon] watching extensions: js,mjs,cjs,json
[SERVER] [nodemon] starting `node src/services/mock/json-server.js`
[SERVER] JSON Server is running
^C

Cancelled after +5 minutes waiting.

Did a git checkout main, then re-ran the tests, finished under a minute.

[TESTS]   (Run Finished)
[TESTS] 
[TESTS] 
[TESTS]        Spec                                              Tests  Passing  Failing  Pending  Skipped  
[TESTS]   ┌────────────────────────────────────────────────────────────────────────────────────────────────┐
[TESTS]   │ ✔  datasets.cy.js                           00:17        9        9        -        -        - │
[TESTS]   ├────────────────────────────────────────────────────────────────────────────────────────────────┤
[TESTS]   │ ✔  query.cy.js                              00:03        2        2        -        -        - │
[TESTS]   ├────────────────────────────────────────────────────────────────────────────────────────────────┤
[TESTS]   │ ✔  upload.cy.js                             00:03        2        2        -        -        - │
[TESTS]   └────────────────────────────────────────────────────────────────────────────────────────────────┘
[TESTS]     ✔  All specs passed!                        00:24       13       13        -        -        -  
[TESTS] 
[TESTS] yarn wait-on http-get://localhost:${PORT}/index.html && cypress run $@ exited with code 0
--> Sending SIGTERM to other processes..
[SERVER] yarn run serve:fuseki exited with code SIGTERM
--> Sending SIGTERM to other processes..
[CLIENT] yarn wait-on http://localhost:${FUSEKI_PORT}/$/ping && yarn run dev exited with code SIGTERM
Done in 39.73s.

I think that's because of the missing default ports?

@afs afs dismissed their stale review April 16, 2024 07:30

Running tests directly with yarn fail on linux

@OyvindLGjesdal
Copy link
Contributor Author

OyvindLGjesdal commented Apr 17, 2024 •
edited

Adding a dependency for dotenv and using both dotenv and crossenv seems to work for the CI-Job, and the variables should be used by default from dotenv when working with it directly.

I don't understand why I have to use cross-env, if I already have the env set, but it feels like the values aren't available if not listed first in the script section, prior to the job. I am clearly missing something here in how this works.

Good news is that the windows job is passing and running tests even when turning on errors again in maven. Bad news is the script is kind of ugly and hackish, and adds an extra dependency (BSD). Will clean up the extra files and unused parts and push to a single commit again hopefully this evening.

Node added a parameter for reading environment vars without using a library in version 20.6, but I don't think it is reachable when using yarn, from web searches.

Another alternative that looks better imo, is writing a simple helper script that sets environment vars (with defaults) and adding it at the beginning. See
https://stackoverflow.com/questions/32446734/how-to-set-environment-variables-in-a-cross-platform-way

That could mean removing a dependency (cross-env) instead of adding a dependency, however I didn't get it to work.

@OyvindLGjesdal OyvindLGjesdal marked this pull request as draft April 17, 2024 19:54
@OyvindLGjesdal
apacheGH-2344: Use run-script-os to write different test:e2e commands…
d8a520f 
… in jena-fuseki-ui

* Enable failure on tests for jena-fuseki-ui maven pom.
* Have separate versions for *nix and windows of test:e2e using run-script-os package
* update lockfile
@OyvindLGjesdal OyvindLGjesdal marked this pull request as ready for review May 26, 2024 22:52
@OyvindLGjesdal
Copy link
Contributor Author

Added some comments #2344 (comment)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kinow kinow kinow left review comments

@afs afs afs left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

yarn run test:e2e fails on MS Windows.
3 participants
@OyvindLGjesdal @afs @kinow