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

test container location #44

Open
sdolenc opened this issue Apr 28, 2020 · 6 comments
Open

test container location #44

sdolenc opened this issue Apr 28, 2020 · 6 comments
Assignees

Comments

@sdolenc
Copy link
Contributor

sdolenc commented Apr 28, 2020

Thank you for merging #43 which gets us closer to testing every package has supports. I'll aim to submit pull requests that get us even closer to 100% in the future.

The Github Actions tests are currently using containers built from my fork of has, but it would be better if we built the containers using this official fork https://github.com/kdabir/has . That way future changes to the official dockerfiles would update the containers we use for testing.

If that sounds okay to you then could you please

  1. Sign in to https://hub.docker.com/ to create a new testing-has repository
  2. Build the four dockerfiles using settings similar to these
    image
  3. Then (after successful builds) updating Github Actions to use the official images instead of the "sdolenc" ones
    container: sdolenc/testing-has:${{ matrix.container }}
    and
    container: sdolenc/testing-has:${{ matrix.container }}

If it helps: I've added you as a collaborator for my repository on https://hub.docker.com

@kdabir kdabir self-assigned this Jun 20, 2020
@kdabir
Copy link
Owner

kdabir commented Jun 20, 2020

Hi,

First off, thanks for detailed steps so that it was easy to follow for me. Never published any containers on dockerhub.

I tried creating the containers using the checked in script, however it failed.
https://hub.docker.com/repository/docker/kdabir/has-test-containers/builds

Do you know what could be the reason?

@sdolenc
Copy link
Contributor Author

sdolenc commented Jun 20, 2020

I'll take a look :) We're using version pinning when installing packages so our tests know which version to expect. I'm guessing a version the dockerfile(s) is using has been removed from the alpine/ubuntu package repository. I'll submit a PR to fix the issue soon

@sdolenc
Copy link
Contributor Author

sdolenc commented Jun 21, 2020

I submitted a pull request #47 which is a proposed change to branch kdabir:change-docker-test-containers. If it looks good and you merge it then it will auto-update your existing pull request #46

I don't anticipate there will be many future issues, but as I mentioned in the description of #47

i'll look into setting up daily builds of my fork's containers so I can catch any future problems

@kdabir
Copy link
Owner

kdabir commented Jun 21, 2020

Hey, thanks for helping here. I have merged the branches. Docker containers are still being built (dockerhub) once that's complete, I hope all the actions will pass.

@sdolenc
Copy link
Contributor Author

sdolenc commented Jun 21, 2020

Awesome! I'm fairly confident the containers will build properly now, but I just discovered two small test failures that I'll submit a pull request for shortly. Sorry

The first fix does not require new container builds. The version of eb that gets installed on ubuntu is 3.18.1, but I set the "expected" version as 3.18.0. Draft fix:
https://github.com/kdabir/has/compare/master...sdolenc:testfix-eb-on-ubuntu?expand=1

The second issue is caused by the new way we're installing npm on ubuntu. I need to add globally installed npm packages to the path so has can see brunch, grunt, gulp, heroku, netlify, and serverless. This fix requires a new build of the ubuntu container.

Update: The draft linked above includes fixes for both issues. I'll verify everything builds and tests pass before submitting the pull request.

@sdolenc
Copy link
Contributor Author

sdolenc commented Jun 21, 2020

tests are frixed with #48. The six npm package tests will pass on ubuntu after a new ubuntu container is built

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants