Skip to content

Add tests to report when package references are out of date #7661

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.

Sign up for GitHub

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

Jump to bottom
Merged
TravisEz13 merged 9 commits into from
Aug 31, 2018
Merged

Add tests to report when package references are out of date #7661

TravisEz13 merged 9 commits into from
Aug 31, 2018

Conversation

@TravisEz13
Copy link
Member

@TravisEz13 TravisEz13 commented Aug 30, 2018 •
edited
Loading

PR Summary

Add tests to report when package references are out of date

  • Also, update the packaging script to generate updated files.wxs

PR Checklist

iSazonov
iSazonov reviewed Aug 30, 2018
View reviewed changes
tools/releaseTools.psm1 Outdated
Copy link
Collaborator

@iSazonov iSazonov Aug 30, 2018

Choose a reason for hiding this comment

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

Seems the message very short to understand in log where it come from.

test/common/package/package.tests.ps1 Outdated
Copy link
Collaborator

@iSazonov iSazonov Aug 30, 2018

Choose a reason for hiding this comment

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

Please add new line.

adityapatwardhan
adityapatwardhan requested changes Aug 30, 2018
View reviewed changes
tools/releaseTools.psm1 Outdated
Copy link
Member

@adityapatwardhan adityapatwardhan Aug 30, 2018

Choose a reason for hiding this comment

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

MMI package is on nuget.org.. we should check for that too.

https://www.nuget.org/packages/Microsoft.Management.Infrastructure/

tools/releaseTools.psm1 Outdated
Copy link
Member

@adityapatwardhan adityapatwardhan Aug 30, 2018

Choose a reason for hiding this comment

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

Should we use semantic version instead, would make the checks much simpler.

$NewVersion = [System.Management.Automation.SemanticVersion]"6.1.0-preview.4"
$Version = [System.Management.Automation.SemanticVersion]"6.0.4"

if($NewVersion.PreReleaseLabel) { return false } else { return $NewVersion -gt $Version } 
…..

Copy link
Member Author

@TravisEz13 TravisEz13 Aug 30, 2018

Choose a reason for hiding this comment

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

I had an offline discussion with @adityapatwardhan . I'll update the synopsis on why the current logic is correct.

tools/releaseTools.psm1 Outdated
Copy link
Member

@adityapatwardhan adityapatwardhan Aug 30, 2018

Choose a reason for hiding this comment

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

Typo tht -> that

@TravisEz13 TravisEz13 force-pushed the improve_new_package_function branch from f70635f to c85916b Compare August 30, 2018 19:08
@TravisEz13
Copy link
Member Author

TravisEz13 commented Aug 30, 2018

@adityapatwardhan @iSazonov I believe I have addressed your feedback

@adityapatwardhan
Copy link
Member

adityapatwardhan commented Aug 30, 2018

@TravisEz13 Please have a look at wix packaging error.

@TravisEz13 TravisEz13 force-pushed the improve_new_package_function branch from 03b098a to 29062ba Compare August 31, 2018 01:46
iSazonov
iSazonov approved these changes Aug 31, 2018
View reviewed changes
Copy link
Collaborator

@iSazonov iSazonov left a comment

Choose a reason for hiding this comment

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

Should we update our Contribution guide?

@TravisEz13 TravisEz13 force-pushed the improve_new_package_function branch from 5b8919f to 31629f9 Compare August 31, 2018 17:21
@TravisEz13 TravisEz13 force-pushed the improve_new_package_function branch from 31629f9 to 9c79fe3 Compare August 31, 2018 18:12
@TravisEz13 TravisEz13 merged commit e2a45f0 into PowerShell:master Aug 31, 2018
@TravisEz13 TravisEz13 deleted the improve_new_package_function branch August 31, 2018 19:51
@TravisEz13
Copy link
Member Author

TravisEz13 commented Aug 31, 2018

This PR is difficult enough to keep up to date. Could you make a PR to update the contribution guide?

@iSazonov
Copy link
Collaborator

iSazonov commented Sep 3, 2018

Done #7695.

TravisEz13 added a commit that referenced this pull request Sep 5, 2018
@TravisEz13
Add tests to report when package references are out of date (#7661)
137d5de 
Add tests to report when package references are out of date
  - Also, update the packaging script to generate updated `files.wxs`
@iSazonov iSazonov mentioned this pull request Sep 6, 2018
Merged
11 tasks
@TravisEz13 TravisEz13 added this to the v6.1.0 milestone Sep 7, 2018
@felixfbecker
Copy link
Contributor

felixfbecker commented Sep 13, 2018

Does this make the build fail if a new version is released, i.e. irreproducable?

@TravisEz13
Copy link
Member Author

TravisEz13 commented Sep 14, 2018

First, let us start with why this test exists. We expect that during development that the latest packages are being used. Unless there is a good reason, not to, at which time we will adjust the tests, if needed.
During development, we expect the latest revision to a package to be used. Comments in the code indicate to disable one of the tests until we have a better mechanism for this.

That is two questions combined into one:

  1. Does this make the build fail if a new version is released?

    • Yes

  2. Is the failure irreproducible?

    • Under certain circumstances the failure would not be reproducible, but that is true for many tests.
      It would not be reproducible if the package owner removed the new package, but the tests logs what the new version was.
      So, this failure would be diagnosable.

The maintainers are happy to fix this issue, if the issue arises in your PR.

@bergmeister bergmeister mentioned this pull request Oct 11, 2018
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@adityapatwardhan adityapatwardhan adityapatwardhan approved these changes

@iSazonov iSazonov iSazonov approved these changes

@anmenaga anmenaga Awaiting requested review from anmenaga

@dantraMSFT dantraMSFT Awaiting requested review from dantraMSFT

@daxian-dbw daxian-dbw Awaiting requested review from daxian-dbw

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

v6.1.0

Development

Successfully merging this pull request may close these issues.

4 participants

@TravisEz13 @adityapatwardhan @iSazonov @felixfbecker