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

NODE - test release candidate #1411

Merged
merged 8 commits into from
May 26, 2024
Merged

Conversation

avifenesh
Copy link
Member

@avifenesh avifenesh commented May 14, 2024

Added testing for release candidate publishing in node

Rebased over #1379 and #1407

Connected to #1177

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

@avifenesh avifenesh requested a review from a team as a code owner May 14, 2024 16:48
@avifenesh avifenesh requested a review from barshaul May 14, 2024 16:49
@avifenesh avifenesh added the CI CI/CD related label May 14, 2024
Copy link
Contributor

@barshaul barshaul left a comment

Choose a reason for hiding this comment

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

See inline comments

@avifenesh avifenesh force-pushed the CD/Feature/test-release-candidate branch from c27c7fd to 3c39c32 Compare May 22, 2024 17:27
@avifenesh avifenesh requested a review from barshaul May 22, 2024 17:28
utils/TestUtils.ts Fixed Show resolved Hide resolved
utils/TestUtils.ts Fixed Show resolved Hide resolved
utils/TestUtils.ts Fixed Show resolved Hide resolved
.github/workflows/npm-cd.yml Show resolved Hide resolved
.github/workflows/npm-cd.yml Outdated Show resolved Hide resolved
.github/workflows/npm-cd.yml Show resolved Hide resolved
.github/workflows/npm-cd.yml Show resolved Hide resolved
.github/workflows/npm-cd.yml Show resolved Hide resolved
utils/release-candidate-testing/node/index.js Outdated Show resolved Hide resolved
utils/release-candidate-testing/node/index.js Outdated Show resolved Hide resolved
utils/release-candidate-testing/node/index.js Show resolved Hide resolved
utils/release-candidate-testing/node/index.js Outdated Show resolved Hide resolved
utils/tsconfig.json Outdated Show resolved Hide resolved
@avifenesh avifenesh force-pushed the CD/Feature/test-release-candidate branch from 886df17 to 1a665c9 Compare May 23, 2024 15:12
@avifenesh avifenesh force-pushed the CD/Feature/test-release-candidate branch 4 times, most recently from bfac1c9 to 7fa8a1c Compare May 23, 2024 16:13
@avifenesh avifenesh force-pushed the CD/Feature/test-release-candidate branch 8 times, most recently from bda3204 to f04e7d6 Compare May 23, 2024 17:06
@avifenesh avifenesh force-pushed the CD/Feature/test-release-candidate branch 10 times, most recently from 3d92e97 to a4eb6df Compare May 24, 2024 09:16
@avifenesh avifenesh force-pushed the CD/Feature/test-release-candidate branch from a4eb6df to 307a1bc Compare May 24, 2024 09:19
@avifenesh avifenesh requested a review from barshaul May 24, 2024 09:22
Copy link
Contributor

@barshaul barshaul left a comment

Choose a reason for hiding this comment

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

Overall LGTM, but left some comments:

  1. Try to see if the matrix can be reformatted to be more readable
  2. Raise detailed error messages in the release-candidate-testing script (including the excepted response and the received one)
  3. Raise the error instead of just logging it - otherwise we won't fail the action when the tests are failing
  4. Some other minor comments

@avifenesh avifenesh merged commit 9216e0f into main May 26, 2024
24 checks passed
@avifenesh avifenesh deleted the CD/Feature/test-release-candidate branch May 26, 2024 10:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants