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

Add test to verify docker images build #5

Merged
merged 23 commits into from
Apr 30, 2023

Conversation

KieranHolroyd
Copy link
Collaborator

@KieranHolroyd KieranHolroyd commented Apr 28, 2023

Currently fails on a number of the test cases, I'm not currently sure how to programatically fix this however, as it seems to be based on the underlying frameworks requiring certain files or folders to exist in order to complete the application build step. It might be worth considering pulling the latest version of whatever "starter project" these test candidates offer, removing any Dockerfile possibly shipped with the starter project, building it with the tool then verifying its correctness.

This pull request is not ready to be merged, I'm interested in feedback as to how this could be made to work easily with possible future test cases in mind.
(it seems my formatter changed a few things, which may go against code style, oops)

Currently failes on a number of the test cases, I'm not currently sure how to programatically fix this however, as it seems to be based on the underlying frameworks requiring certain files or folders to exist in order to complete the application build step.
It might be worth considering pulling the latest version of whatever "starter project" these test candidates offer, removing any Dockerfile possibly shipped with the starter project, building it with the tool then verifying its correctness.
@KieranHolroyd KieranHolroyd marked this pull request as draft April 28, 2023 12:33
@rubys
Copy link
Contributor

rubys commented Apr 28, 2023

The "sister" project to this, dockerfile-rails does start from scratch and uses rails to generate an application. It even includes a single "system test" which launches an application runs the system tests contained within that application. As that is slow, this is only done on one application.

@rubys
Copy link
Contributor

rubys commented Apr 28, 2023

I wonder if the docker setup build action would help?

https://github.com/rubys/dockerfile-rails/blob/main/.github/workflows/system-test.yml#L8-L8

@KieranHolroyd
Copy link
Collaborator Author

KieranHolroyd commented Apr 28, 2023

I certainly had some issues getting docker images to build on actions, but pending this job not exiting early (the test will fail on 5 counts). I think it works properly

The next step would be to find some way of bootstrapping the projects to a buildable state for each test case, as this is currently not the case so the builds fail at the npm run build step or yarn/pnpm equivilant.

Update: it ran fully, however failed after running the tests with exit code 5, I'll be honest and say I have no idea why, this will require more debugging. Got stuff to be doing now however so that will have to wait unless someone else would like to have a go

@KieranHolroyd
Copy link
Collaborator Author

KieranHolroyd commented Apr 29, 2023

Ok, so after some research I found this issue.
It turns out that the exit code 0-255 is the number of failing tests, I tested this and it is apparently the case.

This means that all the previous test cases failing with exit code 5, just have 5 failing tests which is correct, problem solved?

(edit) I also ran this locally and caused 7 failed cases, checking the exit code, it was also 7.

@KieranHolroyd KieranHolroyd marked this pull request as ready for review April 29, 2023 20:36
test/remix-indie/app/root.tsx Outdated Show resolved Hide resolved
@rubys
Copy link
Contributor

rubys commented Apr 30, 2023

Since all tests are passing, merge when you feel comfortable.

@KieranHolroyd KieranHolroyd merged commit 33fc2ae into fly-apps:main Apr 30, 2023
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

Successfully merging this pull request may close these issues.

3 participants