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

[dev-env] e2e Windows tests #1073

Merged
merged 5 commits into from
Aug 18, 2022
Merged

[dev-env] e2e Windows tests #1073

merged 5 commits into from
Aug 18, 2022

Conversation

pschoffer
Copy link
Contributor

@pschoffer pschoffer commented Aug 10, 2022

Description

It would be great to get an automated test that validates dev-env working on windows machines.

Investigating this I ran into an issue with running Linux based images on windows-latest runner. I was not able to figure out how to enable them. So I opened a github discussion

In the meantime, though I think it would be worth adding the creation test which works fine. The way I coded it is very rough, so I would be happy to get any feedback on it.

ping @bsessions85

@pschoffer pschoffer force-pushed the add/pipeline branch 3 times, most recently from 6be0c0c to 6479b08 Compare August 10, 2022 11:13
@mjangda
Copy link
Member

mjangda commented Aug 11, 2022

I looked into this before and one blocker was that GitHub didn't support running docker inside docker. Is that not an issue any more? Or do we have a workaround?

@pschoffer pschoffer force-pushed the add/pipeline branch 2 times, most recently from 7745a62 to adad45a Compare August 11, 2022 08:54
@pschoffer
Copy link
Contributor Author

Hmm I am still fighting with other dev-env issues. But guessing from the fact that windows runner comes with docker and docker-compose preinstalled it might work?

https://github.com/actions/runner-images/blob/main/images/win/Windows2022-Readme.md#tools

@mjangda

@pschoffer pschoffer force-pushed the add/pipeline branch 9 times, most recently from 69035f7 to 0dab219 Compare August 11, 2022 14:15
@pschoffer
Copy link
Contributor Author

It seems to be able to do docker in docker, but I am having hard time convincing it to run Linux containers as the daemon runs in win architecture. I need to read up and try more stuff in order to figure out how to massage it some more.

@pschoffer pschoffer force-pushed the add/pipeline branch 8 times, most recently from d295f89 to a6d9fbe Compare August 12, 2022 09:43
@mjangda
Copy link
Member

mjangda commented Aug 13, 2022

This is what I was thinking of:

actions/runner#904

Copy link
Member

@mjangda mjangda left a comment

Choose a reason for hiding this comment

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

One minor note but this is a very nice improvement 👍

if NOT %errorlevel% == 0 (
echo "== Failed to create dev-env"
exit 1
)
Copy link
Member

Choose a reason for hiding this comment

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

Do we want to check that the config files for the environment were actually created?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good idea. Done.

@pschoffer pschoffer requested a review from mjangda August 15, 2022 12:21
Copy link
Contributor

@rinatkhaziev rinatkhaziev left a comment

Choose a reason for hiding this comment

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

Thanks! Admittedly the usefulness is limited because we can't run the containers.
@bsessions85 any ideas on how can we side-step this?

@pschoffer pschoffer merged commit 278153b into develop Aug 18, 2022
@pschoffer pschoffer deleted the add/pipeline branch August 18, 2022 05:12
@bsessions85
Copy link
Contributor

Thanks! Admittedly the usefulness is limited because we can't run the containers. @bsessions85 any ideas on how can we side-step this?

Thats a good question @rinatkhaziev. I have made it work with linux environment but haven't tried it on Windows before. I just did some quick searching and it doesn't appear to be possible as things stand today. I'll keep thinking through it, perhaps there is a workaround somehow that I'm not thinking of.

@aagam-shah aagam-shah mentioned this pull request Aug 29, 2022
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.

4 participants