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

Use node:16.5 for frontend instead of node:16 #16591

Merged
merged 3 commits into from
Aug 1, 2021

Conversation

zeripath
Copy link
Contributor

@zeripath zeripath commented Aug 1, 2021

Webpack does not appear to work on the latest node 16.6.0 and fails with an inscrutable
message.

I have been unable to work out what the problem is. This PR forcibly uses node 16.5 everywhere instead of node 16.

Close #16592

Signed-off-by: Andrew Thornton art27@cantab.net

Jest does not appear to work on the latest node 16.6.0 and fails with an inscrutable
message.

I have been unable to work out what the problem is. This PR simply disables the
test-frontend part in the makefile.

Another alternative would be to drop node to node 14 - which is the LTS for node.

Signed-off-by: Andrew Thornton <art27@cantab.net>
zeripath added a commit to zeripath/gitea that referenced this pull request Aug 1, 2021
The recent change in node 16 has caused jest to break and thence our CI.
This PR changes to use Node 14 instead of 16.

Closes go-gitea#16591

Signed-off-by: Andrew Thornton <art27@cantab.net>
Signed-off-by: Andrew Thornton <art27@cantab.net>
@zeripath zeripath changed the title Disable frontend testing Use node 16.5 for frontend testing instead of node:16 Aug 1, 2021
@GiteaBot GiteaBot added the lgtm/need 1 This PR needs approval from one additional maintainer to be merged. label Aug 1, 2021
Signed-off-by: Andrew Thornton <art27@cantab.net>
@zeripath
Copy link
Contributor Author

zeripath commented Aug 1, 2021

Ah it might be something to do with webpack

@zeripath
Copy link
Contributor Author

zeripath commented Aug 1, 2021

npx webpack
[webpack-cli] Failed to load '/home/andrew/src/go/gitea/webpack.config.js' config
[webpack-cli] TypeError: String.prototype.startsWith called on null or undefined
    at startsWith (<anonymous>)
    at node:internal/errors:811:19
    at Array.filter (<anonymous>)
    at node:internal/errors:809:16
    at prepareStackTrace (node:internal/errors:96:12)
    at Object.Module._extensions..js (node:internal/modules/cjs/loader:1138:38)
    at Module.load (node:internal/modules/cjs/loader:981:32)
    at Function.Module._load (node:internal/modules/cjs/loader:822:12)
    at Module.require (node:internal/modules/cjs/loader:1005:19)
    at require (/home/andrew/src/go/gitea/node_modules/v8-compile-cache/v8-compile-cache.js:159:20)
make: *** [Makefile:710: public/js/index.js] Error 2

@zeripath zeripath changed the title Use node 16.5 for frontend testing instead of node:16 Use node:16.5 for frontend instead of node:16 Aug 1, 2021
@codecov-commenter
Copy link

Codecov Report

Merging #16591 (a4f3c70) into main (72738f0) will decrease coverage by 0.00%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##             main   #16591      +/-   ##
==========================================
- Coverage   45.35%   45.35%   -0.01%     
==========================================
  Files         750      750              
  Lines       85096    85096              
==========================================
- Hits        38599    38594       -5     
- Misses      40252    40255       +3     
- Partials     6245     6247       +2     
Impacted Files Coverage Δ
modules/notification/mail/mail.go 35.29% <0.00%> (-2.95%) ⬇️
models/unit.go 41.09% <0.00%> (-2.74%) ⬇️
modules/process/manager.go 72.83% <0.00%> (-2.47%) ⬇️
modules/notification/ui/ui.go 60.86% <0.00%> (-1.45%) ⬇️
models/notification.go 64.97% <0.00%> (-0.89%) ⬇️
models/repo_list.go 76.86% <0.00%> (-0.79%) ⬇️
models/error.go 39.07% <0.00%> (-0.47%) ⬇️
services/pull/pull.go 42.13% <0.00%> (+0.40%) ⬆️
services/pull/check.go 26.20% <0.00%> (+0.68%) ⬆️
services/pull/patch.go 55.93% <0.00%> (+1.69%) ⬆️
... and 2 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b73e421...a4f3c70. Read the comment docs.

@zeripath
Copy link
Contributor Author

zeripath commented Aug 1, 2021

OK there's a fix in node already regarding this: nodejs/node#39593

But node 16.7.0 has not been released as yet and is unlikely it appears to be released for a little while yet.

I'm not certain what is the best thing to do:

  • This PR uses 16.5.0 which has a known sec issue - albeit one that doesn't affect us.
  • Use node 14 instead of node 16 as node 14 is LTS #16592 switches us to use node:14 which is the LTS, which does not suffer this problem and should be more stable
  • Or we leave things and wait for the node release.

In the meantime however we can't release, merge or build until this is sorted out.

Personally I vote for merging this and paying attention to node when it releases 16.7 but if another issue like this occurs we should seriously consider if we want to be running the latest version of node instead of the LTS. I think leaving our CI as broken is not a tolerable situation.

@6543
Copy link
Member

6543 commented Aug 1, 2021

agree - get this in and if something similar pop up use LTS :)

@6543 6543 added this to the 1.16.0 milestone Aug 1, 2021
@GiteaBot GiteaBot added lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. and removed lgtm/need 1 This PR needs approval from one additional maintainer to be merged. labels Aug 1, 2021
@lafriks lafriks merged commit e3b6526 into go-gitea:main Aug 1, 2021
@zeripath zeripath deleted the disable-jest branch August 1, 2021 14:33
@6543 6543 changed the title Use node:16.5 for frontend instead of node:16 [CI] use node v14 until v16 will pass again Aug 1, 2021
@6543 6543 changed the title [CI] use node v14 until v16 will pass again Use node:16.5 for frontend instead of node:16 Aug 1, 2021
@6543 6543 added the skip-changelog This PR is irrelevant for the (next) changelog, for example bug fixes for unreleased features. label Aug 1, 2021
AbdulrhmnGhanem pushed a commit to kitspace/gitea that referenced this pull request Aug 10, 2021
* Disable frontend testing

Jest does not appear to work on the latest node 16.6.0 and fails with an inscrutable
message.

I have been unable to work out what the problem is. This PR simply disables the
test-frontend part in the makefile.

Another alternative would be to drop node to node 14 - which is the LTS for node.

Signed-off-by: Andrew Thornton <art27@cantab.net>

* actually just tell on 16.5 instead

Signed-off-by: Andrew Thornton <art27@cantab.net>

* Use node 16.5 instead of 16

Signed-off-by: Andrew Thornton <art27@cantab.net>
@go-gitea go-gitea locked and limited conversation to collaborators Oct 19, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. skip-changelog This PR is irrelevant for the (next) changelog, for example bug fixes for unreleased features. type/testing
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants