Skip to content
This repository was archived by the owner on Jun 13, 2024. It is now read-only.

Enabling meta tests and fixing any findings #423

Closed

Conversation

mrhockeymonkey
Copy link

Hey PowerShell Team,

You kindly merged #347 recently and I in fact never had the chance to add tests. I finally found some time to come back to it but it looks likes @johlju did some amazing work on getting DscResource.Tests integrated into the pipeline. Since I set the time aside I thought i might contribute by enabling some of these tests and fixing some of the issues they raised (Formatting only, nothing functional)

The tests complete fine locally but I'm not sure how to get them running in AppVeyor as a final test, perhaps someone could fill me in on how to do so?

Any feedback welcome, hope this helps

@johlju
Copy link
Contributor

johlju commented Feb 14, 2019

Hi! I’m actually working on this in issue #418, and is almost ready to send in a PR. I have added example publishing, and have clean up the code.
Sorry I haven’t written in the issue that I was actually working on it, and we are working on the same thing now. Been a busy work week so only had a hour here and there to do so that issue.

If I send in a PR tonight or tomorrow of what I’ve done we can compare notes?

You can looo in my working branch in my fork https://github.com/johlju/PowerShellGet/tree/cleanup-psmodule

@johlju
Copy link
Contributor

johlju commented Feb 14, 2019

Btw, we can’t change the open braces, since they will be automatically turned back if someone saves the file in VS Code as the workspace settings kick in.
We should override the rules for that. See example in my fork, link in previous comment.

@johlju
Copy link
Contributor

johlju commented Feb 14, 2019

The meta test run automatically and if it finds the opt-in file it will automatically enable those tests that are opt-in. Just move the file to the correct location.
I had to do some tricking to get the DSC test framework to work since the folder structure in this repo is different from what it expects. So the file need to be added where the code looks for it. Don’t remember top of my head where it should go, but please look at my working branch and see where I put it. 😄

On my phone now on my way to work, so can’t be any more precise right now :)

@johlju
Copy link
Contributor

johlju commented Feb 14, 2019

If you do these changes I think this PR can be merged and I can rebase my working branch on top of that.

@mrhockeymonkey
Copy link
Author

mrhockeymonkey commented Feb 14, 2019

Hey @johlju no worries at all. It wasn't much work and selfishly I was mainly having a stab at this to get my head around how the DSCResource.Tests framework really works in the real world as I want to adopt it internally.

You have a much more intimate understanding of this repo so it sounds like your branch will be a better choice for merging, Im happy to close this off or make these changes, im happy either way

Let me know what you think is best

@johlju
Copy link
Contributor

johlju commented Feb 14, 2019

I’m sure everyone is happy you put in the work on this, and for sure you deserve to get this PR merged. It’s up to you if you want to make the changes. If you make the changes I can review. There is no hurry sending in my PR. So, if you want to put in the work, then please do.
I don’t really have a understanding of this repo, I might just have a bit more knowledge about DscResource.Tests. 😄
And on the subject of DscResource.Tests, glad to here you are trying to adopt it internally. Please submit any questions you have to the DscResource.Tests repo, we are happy to help more people using that.

@johlju johlju mentioned this pull request Feb 15, 2019
@johlju
Copy link
Contributor

johlju commented Feb 15, 2019

@scottmatthews343 I sent in the PR #424 now. It pretty much covers the changes in this PR. 🙂

@alerickson
Copy link
Member

Hi @scottmatthews343, thanks so much for your contributions! I'm looking over @johlju PR right now, but just wanted to confirm what sounds like a consensus to close this PR and merge PR #424. Please let me know if there's anything you'd like to add with this PR, or feel free to create a new PR with changes.

@mrhockeymonkey
Copy link
Author

Yep, all good. Thanks @johlju @alerickson

@mrhockeymonkey mrhockeymonkey deleted the dsc-format branch February 20, 2019 11:14
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants