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

RFC: Add tests for bootstrap-salt.ps1 #893

Merged
merged 2 commits into from
Jul 7, 2016
Merged

RFC: Add tests for bootstrap-salt.ps1 #893

merged 2 commits into from
Jul 7, 2016

Conversation

themalkolm
Copy link
Contributor

DO NOT MERGE AS EVERYTHING IS CONFIGURED TO WORK ON MY REPO

We heavily rely on this script and would like to somehow ensure it won't be broken in future releases. I also would like to fix some minor issues but very reluctant unless there are tests. So here you are, first attempt to make tests using https://www.appveyor.com/

I'm open to comments, suggestions and ideas how it must be structured in a better way. For now it heavily relies on running in AppVeyor and therefore no cleanup logic as AppVeyor will spin up a new worker for every build.

What does this PR do?

Adds AppVeyor configuration enabling automatic smoke tests for bootstrap-salt.ps1. It does not test for all possible cases. Itmostly works as smoke tests to ensure that at least something works. While it sounds strange, the power is in the fact that it runs automatically and has in-built support for matrix builds.

What issues does this PR fix or reference?

It add smoke tests for bootstrap-salt.ps1 and a nice status badge.

@themalkolm
Copy link
Contributor Author

You can check how badge looks like here https://github.com/themalkolm/salt-bootstrap

@themalkolm
Copy link
Contributor Author

You can check how everything looks here https://ci.appveyor.com/project/themalkolm/salt-bootstrap

@rallytime
Copy link
Contributor

What do you think about this @s0undt3ch?

@s0undt3ch
Copy link
Contributor

Some tests is better than no tests...
I'll setup appveyor for this repo in a bit.

@s0undt3ch s0undt3ch merged commit 0980676 into saltstack:develop Jul 7, 2016
@s0undt3ch
Copy link
Contributor

Woops, missed the no merge part.

Want to fix the links to your own repo as a new PR?

@themalkolm
Copy link
Contributor Author

@s0undt3ch sure, but could you provide me a link to appveyor project you created? Can't find. You also need to configure it to build stable, develop & pull requests I believe.

Then we need to agree (would be nice to do it explicitly) that badge should show current status of the develop branch. Having this badge rises the next question if we should show travis badge as well and enable travis pull requests too.

@s0undt3ch
Copy link
Contributor

https://ci.appveyor.com/project/saltstack-public/salt-bootstrap

As for badges, I'd like to leave them out for now because enabling Travis for the Linux side will give a false sense of a working script because it only tests Ubuntu.

There are plans to cover all distributions the script supports but its not an easy task and we don't currently have the free time to work on this. But its not forgotten and the SaltStack QA team does run a huge batch of manual tests before releasing the script.

@themalkolm themalkolm mentioned this pull request Jul 9, 2016
@themalkolm
Copy link
Contributor Author

@s0undt3ch well, not like we test all Windows versions here too ...

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.

3 participants