-
Notifications
You must be signed in to change notification settings - Fork 22
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
Discussion: Add / validate support for Linux / MacOSx #34
Comments
I think that's a very good idea. I guess we could take the same Tests and just run them on other (Gitlab Term) "Runners". Don't know how they are called on appveyor ^^ I would also see this as a possibility to change the CI to a build Script to clean up the yml file. |
Suggesting two ways.
|
I had to make one small correction to get the module to load on Linux. I didn’t actually test anything on it. Have you looked at Azure DevOps for CI? You get Windows, Linux and MacOS testers. I have a project I’ve started and I run the same scripts to test all three platforms. I’ll do some testing in the next few days and see how it goes. |
Thanks for your feedback guys 👍 ! Indeed @bateskevin redoing the CI is something that I wanted to do anyways. It make sense to do that now. If I summarize our thoughts up until now:
I do have something localy for another test repo, which I can reuse and combine. I'll take this one, and start working on it immediatley, and come back here to share my progress and ask for your valuable feedback 🦄 😸 |
This change was required for the module to load. The issue stems from the case sensitivity of the Linux file system -
I ran your pester tests on my laptop, there were a lot of failures. Looks like something you use is either not available on PowerShell core or needs to be reworked. I will do some investigations. I've attached the output file from the pester run. |
I'm fairly new at this CI stuff for PowerShell, I created a HelloWorld project to experiment with. I've not gotten to the publishing piece, and I've pulled together things I've seen on other projects. |
I've found another issue, it just didn't throw an error till I loaded with -verbose
In pscore they don't created aliases that cover native commands - so my guess is changing sort to sort-object will fix most of the issues. I will try later |
Changing sort to Sort-Object was the trick. Now only 8 Pester tests fail. Best I can tell all are related to PSGraph not looking for graphviz where it got installed - I think it is an easy fix, and will do some testing and either open an Issue or PR to that project. Here is the updated
I've also attached the latest |
A simple correction on PSGraph to find graphviz and it's down to two test failed. I'll open an issue on the PSGraph project and see if Kevin wants to fix it or if I can do a PR for it. Attached is my current |
Hey @Stephanevg Would you mind if I submitted a PR to make the changes listed above. I'm needing two more PR for #hacktoberfest |
of course! please do!
Note i have done some changes on the CI recently, so you might want to
integrate them first using 'git pull' first.
Le 12 oct. 2018 00:44, "Mark" <notifications@github.com> a écrit :
Hey @Stephanevg <https://github.com/Stephanevg>
Would you mind if I submitted a PR to make the changes listed above. I'm
needing two more PR for #hacktoberfest
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#34 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AGAs8fs1R8MbV-gc6mpQlfmdlHuBnEkNks5uj8ncgaJpZM4XLAvM>
.
|
Will do that. |
PR #43 submitted. All but 3 Pester tests are now passing on Linux. All three tests that are failing are in Tests/ClassUtils.Write-CUClassDiagram.Tests.ps1. The The commands being tested are working, the output just doesn't match the regular expression. I will need to do some testing on Windows to compare the output to see what isn't working. |
thank you so much for your PR @tigerfansga I just merged it in dev. I will update the CI part to have a build on linux as well, and merge it when everything is green there as well. Concerning your issues with the 3 remaining pester tests. There are perhaps a few things you can look into (If you want of course :)) For some reason, I have the impression this is a casesensitivty thing, again . there are some hints that could (maybe) spot the issue: We can add ignore case in the call, to force the cmdlet to completley ignore the casing (this forces everything to lowercase actually) (note also, I lowered cased the w from the cmatch.
You could look into the variable $b (simply output it to the console) and you will see the string that it contains. Somewhere, it should contain Cheers |
Actually, my first thought was a case thing but I ran basically manually did the tests and there was output that looked valid but didn't have anything close to matching the regular expression. I got a memory upgrade for my laptop and will be able to run a windows vm. I will run a similar test and compare the output between the two platforms to see. I'm just not familiar enough with what the output looks like when run on windows to make a judgement. |
Progress. I was able to setup a windows VM on my laptop to do some testing. Working with the current master branch, I was able to get everything setup and run the Pester tests. On Windows PowerShell, all tests pasted as expected. On PowerShell Core, two of the tests with that are failing on Linux also fail. So there is some kind of behavior change between Windows PowerShell and PowerShell core. I will research more later. |
great! i have the impression we are close to complete this one! |
I will run the test manually and post the output |
Ok, I've recreated the issue. Ultimately, it looks like somethings are getting ordered differently breaking the regex. It definitely looks like an issue with the test and not the actual code. So I created a file
Then on Windows, I ran the following command in both Windows PowerShell and PowerShell Core In Windows PowerShell here is the output
In PowerShell core
The line that is causing the issue in the test is this PowerShell Core |
I suggest making the following change to the test regex (it appears 3 times) from Can probably make the regex a little tighter but I'll start with that. Also, had a new file system case issue pop up from the CI restructuring. It only effects the tests actually, but I'll work it into the next pull request. |
I've submitted PR #44 to correct these issues. I ran Pester test locally on Linux and Windows (Windows PowerShell and PSCore). This should get the module in good shape. I do have access to a MacOS machine and will test in the next week. |
wow, @tigerfansga ! I wasn't expecting that level of detail, great work! I have merged your branch into dev, and will shortly add it into master. I still think it is a bit strange, that the psgraph returns another graph result on linux. Have your tried running the following command on linux? #sample.ps1 contains the classes from the example in the discusion above
Write-CUClassDiagram -Path .\sample.ps1 -show theoretically, it 'should' create the following output (Except the title of the graph is called 'inheritence' here, but it should be sample.ps1: if not, we have a little bit more work on the write-CUClassDiagram to do to make it 'linux friendly' then what I originally thought. |
I have updated our CI on this project now. From now on, we will have a build on Windows, and linux each time we push a commit to one of the branches. I think @tigerfansga might find this usefull 😉 I could see we have some issues with the packageProvider, and one pester test failing for Write-CUClassDiagram. @tigerfansga have you seen this one before? |
Actually, the difference is Windows PowerShell verse powedhell core. The output was all windows. I think the graph actually is the same, just things ordered differently. |
Hi @tigerfansga
Do you remember what you did to fix it ? |
Hi @tigerfansga I actually got around, and found your PR KevinMarquette/PSGraph#78 , which seems not be applied in version 2.1.33. We are one failing test away from closing this issue :) |
Tracking the last issue here --> #71 |
I have removed linux testing support from the CI (see d2e809c) as I think that PSGraph has an issue on Linux, where we cannot do much. This also means, that PSClassUtils currently doesn't support Linux officially. |
1 similar comment
I have removed linux testing support from the CI (see d2e809c) as I think that PSGraph has an issue on Linux, where we cannot do much. This also means, that PSClassUtils currently doesn't support Linux officially. |
Edit
All issues related to Linux support are tracked here --> https://github.com/Stephanevg/PSClassUtils/projects/5
Hi all,
I think it is about time we takle the cross platformness of this module.
I belive we dont have anything that is blocking us from running this module on Linux / MacOSx, but I can't say for sure. I know that @tigerfansga has been using the module on Linux, but unsure on how sucessfully he was (You perhaps have some info to share with us?)
It would be great if someone could try this module out on a linux and/or macos box.
Perhaps, we could also add a Pester test or two, which would test if the module works on Windows, Linux and MacOSx.
Perhaps this means changing the CI? I am open for the discussion. @bateskevin Any insights on this one?
The text was updated successfully, but these errors were encountered: