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

Bump up github actions for typescript model generation workflow #1343

Merged
merged 5 commits into from
Nov 13, 2023

Conversation

thepetk
Copy link
Contributor

@thepetk thepetk commented Nov 9, 2023

What does this PR do?

This PR bumps up & pins to their latest version the following github actions:

  • actions/setup-python
  • py-actions/py-dependency-install
  • actions/setup-node

This can be a potential fix to the currently failing action of release-typescript-models.yaml workflow, as it updates the version of node used from 12 to 16 and now the yarn build command will not have the invalid version error. More info about its trace here: https://github.com/devfile/api/actions/runs/6800556156/job/18489362645

Finally the PR fixes the codecov workflow failure (can be found here) by updating the setup-go action inside the code coverage workflow to use go 1.18 exactly like we do in ci.yaml.

Which issue(s) does this PR fix

fixes #1232

PR acceptance criteria

Testing and documentation do not need to be complete in order for this PR to be approved. We just need to ensure tracking issues are opened.

  • Open new test/doc issues under the devfile/api repo
  • Check each criteria if:
  • There is a separate tracking issue. Add the issue link under the criteria
    or
  • test/doc updates are made as part of this PR
  • If unchecked, explain why it's not needed

How to test changes / Special notes to the reviewer

The release-typescript-models workflow has been tested here: https://github.com/thepetk/devfile-api/actions/runs/6814583173/job/18531772679

Another way to reproduce locally the error and to see the suggested workaround:

Using node 12 (current status)

$ nvm use 12
$ bash ./build/typescript-model/generate.sh
...
yarn install v1.22.19
info No lockfile found.
[1/4] Resolving packages...
[2/4] Fetching packages...
[3/4] Linking dependencies...
[4/4] Building fresh packages...
success Saved lockfile.
$ npm run build
[#######################################] 39/39npm ERR! Invalid version: "false"

Using node 18

$ nvm use 18
$ bash ./build/typescript-model/generate.sh
...
yarn install v1.22.19
info No lockfile found.
[1/4] Resolving packages...
[2/4] Fetching packages...
[3/4] Linking dependencies...
[4/4] Building fresh packages...
success Saved lockfile.
$ npm run build
[#######################################] 39/39
> @devfile/api@false build
> tsc

Done in 8.09s.
yarn run v1.22.19
$ tsc
Done in 3.84s.

Regarding the code coverage report you can test locally by using goenv:

go version 1.17

$ goenv global 1.17.13
$ go test ./... -coverprofile cover.out
...
vendor/k8s.io/apimachinery/pkg/util/sets/set.go:24:12: too many errors
note: module requires Go 1.19

go version 1.18

$ goenv global 1.18.10
$ go test ./... -coverprofile cover.out
...
# runs ok

Copy link

codecov bot commented Nov 9, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (b4fea57) 35.75% compared to head (62ffb5a) 35.75%.
Report is 1 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #1343   +/-   ##
=======================================
  Coverage   35.75%   35.75%           
=======================================
  Files          52       52           
  Lines        6696     6696           
=======================================
  Hits         2394     2394           
  Misses       4158     4158           
  Partials      144      144           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@thepetk thepetk marked this pull request as ready for review November 9, 2023 16:47
@thepetk thepetk changed the title WIP: Bump up github actions for typescript model generation workflow Bump up github actions for typescript model generation workflow Nov 9, 2023
@openshift-ci openshift-ci bot added lgtm and removed lgtm labels Nov 9, 2023
@thepetk
Copy link
Contributor Author

thepetk commented Nov 9, 2023

As nodejs version 16 is EOL I'm updating the version of setup-node -> node-version:18. I was able again to generate the typescript models (with version 18) and I've also updated the description of the PR.

cc @amisevsk (I've re-requested a review as the PR has been updated)

@thepetk thepetk requested a review from amisevsk November 9, 2023 21:21
Copy link
Member

@rm3l rm3l left a comment

Choose a reason for hiding this comment

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

Which issue(s) does this PR fix
fixes #1232

#1232 also talks about fixing the code coverage report workflow, which is still failing with this PR.
I guess you also need to update the version of Go used in the workflow, right? Otherwise, #1232 would not be completely fixed by this PR..

@thepetk
Copy link
Contributor Author

thepetk commented Nov 10, 2023

Which issue(s) does this PR fix
fixes #1232

#1232 also talks about fixing the code coverage report workflow, which is still failing with this PR. I guess you also need to update the version of Go used in the workflow, right? Otherwise, #1232 would not be completely fixed by this PR..

I wanted only to fix the typescript part as it was mentioned as a blocker. But yeap, I think I can dig a little bit an check the coverage too.

@thepetk
Copy link
Contributor Author

thepetk commented Nov 10, 2023

I wanted only to fix the typescript part as it was mentioned as a blocker. But yeap, I think I can dig a little bit an check the coverage too.

@rm3l I've bumped up the go-version used inside the .github/workflows/codecov.yaml to use 1.18. This way we are aligning it with the setup-go action inside the ci.yaml where we run the codecov too.

I'd say that bumping up to 1.19 is more related to the scope of #1237. wdyt?

@thepetk
Copy link
Contributor Author

thepetk commented Nov 10, 2023

I've updated the description of the PR with information about the coverage report fix and instructions on how to test it locally :)

Copy link
Member

@rm3l rm3l left a comment

Choose a reason for hiding this comment

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

Added a few comments.

thepetk and others added 5 commits November 10, 2023 14:57
Signed-off-by: thepetk <thepetk@gmail.com>
Signed-off-by: thepetk <thepetk@gmail.com>
Signed-off-by: thepetk <thepetk@gmail.com>
Signed-off-by: thepetk <thepetk@gmail.com>
Signed-off-by: thepetk <thepetk@gmail.com>

Co-authored-by: Armel Soro <armel@rm3l.org>
Signed-off-by: thepetk <thepetk@gmail.com>
@thepetk
Copy link
Contributor Author

thepetk commented Nov 10, 2023

Added a few comments.

@rm3l I've added the suggested changes :)

Copy link

openshift-ci bot commented Nov 10, 2023

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: amisevsk, rm3l, thepetk

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@thepetk thepetk merged commit 447aa00 into devfile:main Nov 13, 2023
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Release typescript models and code coverage report workflows are failing
3 participants