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

Tests improvments and Linux support #668

Merged
merged 15 commits into from
Aug 29, 2019
Merged

Conversation

ili101
Copy link
Contributor

@ili101 ili101 commented Aug 29, 2019

Integrated https://github.com/ili101/Module.Template. This is a simple template of a module with examples on how to integrate Pester tests and AppVeyor and Azure DevOps Pipelines. this bring:

  • AppVeyor and Azure DevOps Pipelines tests on Windows, Linux and Mac (PowerShell, PowerShell Core and PowerShell Core preview).
  • Test badges per test:
    image
  • Test results published so you can see what filed and passed from GitHub, AppVeyor and Azure:
    image
  • Creates Artifacts in AppVeyor and Azure with the module ready to be installed.
  • You can optionally set up manual publish to the PowerShell Gallery in Azure DevOps from the Artifact.
  • You can F5 from VSCode to run the tests (Select in the Debug Tab).

Code Cleanup

  • Fix casing in some of the files names.
  • There are 3 files lists in: Install.ps1, ImportExcel.psm1, and filelist.txt that needed to be compared and fixed.
  • Tests for features that are not supported yet by EPPlus or by PowerShell Core will be marked as Pending.
  • If a test cannot run like if Excel is open it will be marked as Inconclusive.

Linux Support

  • In Export-StocksToExcel.ps1 replaced $env:TEMP with [IO.Path]::GetTempPath() for Linux support.
  • In all tests replace $env:TEMP with TestDrive:. It's a spacial drive created by the Pester module to help manage the files used by the tests, It also automatically cleans after exiting the scope and is cross-platform.
  • As some of the tests use Get-Service, Get-CimInstance and Get-Process that are missing or different on Linux I added Sample data so the test can be run on any platform.

PS
I noticed that there are 2 orphaned files Export-charts.ps1 and GetExcelTable.ps1 that are not referenced anywhere, not sure if intentional or just forgotten to be added to the psm1.

All tests are now running on Linux and Windows I relay dislike "returning" from a test without at last reflating it in the Pester summary, If something is wrong I don't want to see 100% Passed tests. Same goes to skipping the test if Excel is open that now show as "Inconclusive" in the summery.

Regarding #666 Skipping all AutoSize tests in Linux, I didn't see any problem with AutoSize on Linux, not sure why it was done, all tests are passing now on Linux to.
None of the tests are now skipped on Linux.
I was wandering why TestDrive: is not used well before the Linux support issue, I think it's a cleaner solution.

@dfinke Regarding your Docker enhancement request, do you think it's something that have a demand for? I didn't see people using modules form Docker. I'm going to check it out anyway as I didn't use Docker before and you piqued my interest (:

@dfinke
Copy link
Owner

dfinke commented Aug 29, 2019

@ili101 Good stuff!

For Docker, I've been thinking it'd be interesting if I and others could pull an image down, run it and try out the the module, remove it, all without polluting the main box. If it's easy to build that sort of pipeline, then we can experiment to see if it has value.

The Azure DevOps fails, strange.
https://dougfinke.visualstudio.com/ImportExcel/_build/results?buildId=483&view=logs&j=fd490c07-0b22-5182-fac9-6d67fe1e939b&t=dac0b392-2308-5704-b9f7-c5b20adf012e&l=9

@ili101
Copy link
Contributor Author

ili101 commented Aug 29, 2019

Yes It looks like it run the azure-pipelines.yml from your master and not from the PR.
I don't know if this is a security feature that will use the new azure-pipelines.yml only after merge or maybe you have your Build hard coded and not set to use the yml? can you check in Pipelines > Builds and see if it set to use the yml or maybe create new from yml? I can't check as I have read permission.

@dfinke
Copy link
Owner

dfinke commented Aug 29, 2019

Good point, I set that up a while ago using tasks, not yaml. Looks like I need to delete it and setup another pipeline to just the yaml.

@ili101
Copy link
Contributor Author

ili101 commented Aug 29, 2019

Ok. ping me when you have the yaml pipeline active and I will push something in here to retrigger the test.

@ili101
Copy link
Contributor Author

ili101 commented Aug 29, 2019

Update regarding the AutoFitColumns() problem from #666. I just tested it on WSL and also got the problem.
This is fixed by running sudo apt-get install -y --no-install-recommends libgdiplus libc6-dev (Solution from VahidN/EPPlus.Core#40)
Surprisingly this didn't came out on AppVeyor and Azure, maybe it's already preinstalled there.

@dfinke
Copy link
Owner

dfinke commented Aug 29, 2019

Yeah, saw EPPlus needed that lib from there nuget package. Could be that it is installed. Maybe ImportExcel needs a pre-flight check now?

@ili101
Copy link
Contributor Author

ili101 commented Aug 29, 2019

There is actually one test in the psm1 already:
Write-Warning 'PowerShell 5 is required for plot.ps1'
Added 2 more for Ubuntu and Mac below it:

Write-Warning -Message 'ImportExcel Module Cannot Autosize. Please run the following command to install dependencies: "sudo apt-get install -y --no-install-recommends libgdiplus libc6-dev"'
Write-Warning -Message 'ImportExcel Module Cannot Autosize. Please run the following command to install dependencies: "brew install mono-libgdiplus"'

@dfinke
Copy link
Owner

dfinke commented Aug 29, 2019

@ili101 I setup a new pipeline, only yml, only PS 5.1. Could you poke the PR to see if it hits the same issue?

@dfinke
Copy link
Owner

dfinke commented Aug 29, 2019

@ili101 Thanks, looks like it is working.

@ili101
Copy link
Contributor Author

ili101 commented Aug 29, 2019

Looks good. The paused Pipeline "ImportExcel-CI" is still trying to run the test to. Maybe disable it and cancel the 2 test that are queued there.

@dfinke
Copy link
Owner

dfinke commented Aug 29, 2019

@ili101 Alright, going merge, a little apprehensive. Thanks for the work, really appreciate it.

@dfinke dfinke merged commit 29efd50 into dfinke:master Aug 29, 2019
@dfinke
Copy link
Owner

dfinke commented Aug 29, 2019

@ili101 Merged, I updated the badges. I turned on the new Az DevOps preview, not sure how to index into each job.

This looks great. What's the best way to chat with you outside of GitHub?

@ili101
Copy link
Contributor Author

ili101 commented Aug 29, 2019

appveyor badge need update to:

[![Build status](https://ci.appveyor.com/api/projects/status/21hko6eqtpccrkba/branch/master?svg=true)](https://ci.appveyor.com/project/dfinke/importexcel)

Skype? ili1011

@ili101
Copy link
Contributor Author

ili101 commented Aug 31, 2019

@dfinke After learning to use Docker turned out it was very simple to set up especially because I already have the Artifact ready.
Take the Dockerfile from https://github.com/ili101/Module.Template/tree/master/Tests (uncomment the libgdiplus fix) and put it in your Tests folder.
Then import the Azure Docker config.json and follow the Publish to Docker Hub instruction in https://github.com/ili101/Module.Template/blob/master/README.md.

@ili101 ili101 deleted the TestsRebase branch September 1, 2019 15:11
@ili101
Copy link
Contributor Author

ili101 commented Sep 25, 2019

This looks great. What's the best way to chat with you outside of GitHub?

@dfinke I started using twitter recently, my user is https://twitter.com/ili_z please add me so we can chat if needed. Thank you.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants