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

chore: add smoke tests to the images #1197

Merged
merged 1 commit into from
Feb 13, 2020
Merged

chore: add smoke tests to the images #1197

merged 1 commit into from
Feb 13, 2020

Conversation

SimenB
Copy link
Member

@SimenB SimenB commented Jan 14, 2020 •
edited
Loading

Second part of #1195 (comment)

@SimenB SimenB requested a review from a team January 14, 2020 09:21
This was referenced Jan 14, 2020
Merged
Closed
@tianon
Copy link
Contributor

tianon commented Jan 14, 2020

IMO this is a good idea regardless (as I noted over there) 👍

PeterDaveHello
PeterDaveHello reviewed Jan 15, 2020
View reviewed changes
Copy link
Member

@PeterDaveHello PeterDaveHello left a comment

Choose a reason for hiding this comment

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

Let's also remove npm --version & yarn --version in test-image.bats? Maybe also move node -e "process.stdout.write(process.versions.node)" here?

BTW, I'm not sure if the smoke tests comments are really needed, maybe not.

@SimenB
Copy link
Member Author

SimenB commented Jan 20, 2020 •
edited
Loading

Good call.

EDIT: Wait, maybe not. bats runs against the built images, which is different than running from inside the build process. Not a big difference of course

@nschonni nschonni mentioned this pull request Jan 20, 2020
Merged
@nschonni
Copy link
Member

This could probably land as-is and decide about the BATS after. I think it makes sense to remove it for these cases, but this can help test some failures happening for ARM right now

@SimenB
Copy link
Member Author

SimenB commented Feb 13, 2020

Yeah, let's

@SimenB SimenB merged commit eb89a38 into nodejs:master Feb 13, 2020
@SimenB SimenB deleted the smoke-test branch February 13, 2020 21:41
@nodejs-github-bot nodejs-github-bot mentioned this pull request Feb 13, 2020
Closed
@nodejs-github-bot
Copy link
Collaborator

Created PR to the official-images repo (docker-library/official-images#7472). See https://github.com/docker-library/faq#an-images-source-changed-in-git-now-what if you are wondering when it will be available on the Docker Hub.

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

@PeterDaveHello PeterDaveHello PeterDaveHello left review comments

@LaurentGoderre LaurentGoderre LaurentGoderre approved these changes

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

Successfully merging this pull request may close these issues.
6 participants
@SimenB @tianon @nschonni @nodejs-github-bot @LaurentGoderre @PeterDaveHello