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

(GH-30) Add ability to compare meta information of found binaries #35

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

AdmiringWorm
Copy link
Member

@AdmiringWorm AdmiringWorm commented Oct 5, 2020

Description

This pull requests adds the ability to store and compare basic information of the included binary files in each package version.

Related Issue

fixes #30

Motivation and Context

To be able to easier notice the changes regarding binary files between each version.

How Has This Been Tested?

Currently only been tested in a remote devcontainer in vscode.

Screenshots (if appropriate):

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • My code follows the code style of this project.
  • My change requires a change to the documentation. (maybe?)
  • I have updated the documentation accordingly.
  • I have read the CONTRIBUTING document.
  • I have added tests to cover my changes. (not added yet, need help to figure out how to test the changes)
  • All new and existing tests passed.

Additional

This pull request is currently a WIP, unit tests have not yet been added nor completely tested appropriately.
Wanted to get some feedback on the current implementation before going any further with this one.

example output:

PS /workspaces/chocodiff> cat /tmp/chocodiff/7zip.install.18.6/binary-data.txt


Name                           Value
----                           -----
path                           ./tools/7zip_x32.exe
checksum                       7A2E78C5C2EBC072088B7D8DFF6E00D414E7084C97C2C730CC4C45905668A8F3
CompanyName                    
fileVersion                    
productVersion                 
path                           ./tools/7zip_x64.exe
checksum                       18D0BF6A9A2BC4DD0779B5FCEFE0AA893F938419D755FE9BC6DBFA50D94BD488
CompanyName                    
fileVersion                    
productVersion 

@codecov-commenter
Copy link

codecov-commenter commented Oct 5, 2020

Codecov Report

Merging #35 into master will decrease coverage by 7.40%.
The diff coverage is 17.64%.

Impacted file tree graph

@@             Coverage Diff             @@
##            master      #35      +/-   ##
===========================================
- Coverage   100.00%   92.59%   -7.41%     
===========================================
  Files           14       14              
  Lines          173      189      +16     
===========================================
+ Hits           173      175       +2     
- Misses           0       14      +14     
Impacted Files Coverage Δ
...ocolatey-diff/Public/Get-ChocolateyPackageDiff.ps1 82.71% <17.64%> (-17.29%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 63ce723...89b6799. Read the comment docs.

@AdmiringWorm
Copy link
Member Author

@mkevenaar would you have the possibility to take a look at the current implementation in this draft?

It is not completely ready yet, but I would like to get some feedback before going any further with this.

Sorry about the changes to the formatting, this was a mistake due to automatic formatting in vscode. I will revert those changes when the PR is ready

@@ -107,6 +107,31 @@
Expand-Archive -Path $oldFileName -DestinationPath $oldExtractPath -Force
Expand-Archive -Path $newFileName -DestinationPath $newExtractPath -Force

foreach ($path in @($oldExtractPath; $newExtractPath)) {
# Only because of unit tests
if (!(Test-Path $path)) { continue; }
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this there? What is the use of this?
I think you'll need to create Mocks to get around this.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't remember right now, I have had this code in a stash for quite some time.
I remember that the unit tests were failing until I added this statement, but the unit tests have been updated since the initial stash creation so it may not be needed any longer.

The comment was also more a reminder to myself to figure out why the unit test was failing.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The Pester tests mock the folder and should not actually go out on the internet to download an actual package. I guess it's due to that.

You'll do need to make actual Pester tests.

chocolatey-diff/Public/Get-ChocolateyPackageDiff.ps1 Outdated Show resolved Hide resolved
@AdmiringWorm
Copy link
Member Author

I have moved most of the relevant code to a new function.
Still need to figure out how to do the unit tests (and especially the mocking).

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.

Create a small meta file with information about information regarding binary files
3 participants