Suggestion: Use AppVeyor for CI

[robbinsrush]

!NOTE! to future people looking in: The AppVeyor links may be outdated due to me redoing the project to test things.

I tested a fork of this project on my profile and without dealing with any .yml stuff, was able to build the project and run all Unit Tests. Sadly, to get past the current failing errors I had to disable testing, but we're still able to get automatically generated release .zips of the project with all tests ran with every commit.

If you want to take a peak on how it looks on my setup:

With the failed build:

https://ci.appveyor.com/project/robbinsrush/doomlauncher/build/1.0.1

With the tests disabled and set to upload the release .zip as an artifact to the site:

https://ci.appveyor.com/project/robbinsrush/doomlauncher/build/1.0.3

Edit: Or you can just release it straight to GitHub releases

[nstlaurent]

This looks cool, thanks for posting. I wonder why those unit tests are failing on their server. They both have to deal with files but they are not the only tests that deal with creating/modifying files. Could be the classic "works on my machine" problem...

Will have to look into this.

[guynamederick]

Interesting how it's only the CNDoom test that is failing, and I seem to get the same error on the UnitTest myself as well. On the bright side, all the other tests are working good, so no vital features or anything are buggy to say the least.

[robbinsrush]

I was trying to see if I could fix those failing tests (because both of those failed on my machine before I even touched that CI) last night and it got ugly. I think there may be an issue with the Cndoomstats reader, or maybe something about the test method. I'm leaning towards the reader itself

And the file modified also failed on my machine, but sometimes worked and didn't. I gave up.

[nstlaurent]

Yeah this one has me stumped. Even worse I can't help in figuring out why they fail is because the unit tests never fail on my computer.

[robbinsrush]

Aw geez. Maybe make sure there's no diffs inbetween the github version and your local version on both the test and the StatReader? Maybe even DoomLauncher.DataSources. This is weird, to say the least.

Doing a fresh download shows only CnDoom being the bad one.

[guynamederick]

Debugging on my side, the CNDoomStatsReader is actually working as intended, so it's definitely not the reader when using a build. I can't seem to find any other error to indicate what is wrong with the test file.

The Boom test stats file is tested with a similar method and passes, so for this one to not pass with AppVeyor CI is odd. I have doubts there is some other package or component for Visual Studio that would need to be installed for this one to pass.

[robbinsrush]

@guynamederick Humor me, see if doing a fresh download of the repo and going straight to Run All Tests causes any tests to fail.

And we can set scripts to run on the CI to make sure that certain files are placed in specific places or whatever is needed to make these tests pass.

[guynamederick]

@robbinsrush Yet again the test file for CNDoom failed on me, not sure what you were expecting. I also tried repairing Visual Studio 2017 and whatever I thought I was missing but no luck. And by what you're saying, skipping the CNDoom stats file would sound like the best option at the moment, the error is really odd to figure out here.

[guynamederick]

@robbinsrush Update, got it working! Try out this file, and if it works for you, I'll do a commit.
test_cndoom.zip

[robbinsrush]

Hot diggity damn, what do you know.

And the File Modified one passed, too. I'll send this over to the CI and see how it reacts.

[guynamederick]

And all I did was used a new stdout.txt file and pasted it there...coding is fun. :)

Alright, I'll be doing a commit on the development branch. Glad to hear it worked!

[robbinsrush]

And the CI still is unhappy.

groan

robbinsrush@8f2c5f5 The commit it tried to build.

[guynamederick]

Arrrghh, and I thought this would get by. This "bug" is really just not worth working over, seems like TestCnDoomStats should be excluded from testing for CI, nothing is making sense here.

[robbinsrush]

I'll send in a pull with those ignored. RIP in pep.

@hobomaster22 Let us know if this goes against your moral code to skip Unit Tests 😄

If you need help setting up the CI, I'll gladly help.

Edit: #48

[nstlaurent]

@robbinsrush It does go against my moral code, but in this case I think it's best to skip them for now. I will create another issue on these unit tests specifically.

[nstlaurent]

I installed VS on my desktop and luckily both these unit tests failed. They should be fixed on this branch. If you guys could confirm they work for you too that would be great!

https://github.com/hobomaster22/DoomLauncher/tree/bug/unit_test

[robbinsrush]

Full pass first try on CI:

https://ci.appveyor.com/project/robbinsrush/doomlauncher/build/1.0.1

And full pass first try on my local machine:

I'll have to scroll through your commits to see what you did, but well f'in done, hobomaster.

EDIT: Only thing I would change in your improvements is make the Thread.Sleep(1000) command go for an extra millisecond, because you never know if it will barely make it in that small window.

Other than that, full props.

[guynamederick]

It passed. What you did there to make it work is just amazing, that CnDoomStatsTest can't annoy anybody now. Looks like the unit tests should be good to go.

[guynamederick]

@robbinsrush With this out of the way, you won't mind setting up the AppVeyor CI yourself and doing a pull request for that? Seems to me you're getting that set up. :)

[robbinsrush]

I'd happily do that but I'd need to be a contributor like you are. And I think that can only be done by @hobomaster22

It's not a pull requestable thing