Skip to content

change 'job Test -Safe' to '?Test' #595

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

Merged
merged 6 commits into from
Jan 3, 2018
Merged

change 'job Test -Safe' to '?Test' #595

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
9cbd2ca
change 'job Test -Safe' to '?Test'
TylerLeonhardt Jan 2, 2018
3b87764
see if AppVeyor can handle no required version
TylerLeonhardt Jan 2, 2018
781df1c
add maximum versions
TylerLeonhardt Jan 2, 2018
a9e3b7c
maximum, not required. typo
TylerLeonhardt Jan 2, 2018
1dc3f74
switch platyps back to requiredversion
TylerLeonhardt Jan 3, 2018
b3d067a
consistant platyps version
TylerLeonhardt Jan 3, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion PowerShellEditorServices.build.ps1
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -169,7 +169,7 @@ task TestHost -If { !$script:IsUnix} {
exec { & $script:dotnetExe xunit -configuration $Configuration -framework net452 -verbose -nobuild -x86 }
}

task CITest (job Test -Safe), {
task CITest ?Test, {
# This task is used to ensure we have a chance to upload
# test logs as a CI artifact when the tests fail
if (error Test) {
Expand Down
4 changes: 2 additions & 2 deletions appveyor.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,8 @@ install:
Install-PackageProvider -Name NuGet -MinimumVersion 2.8.5.201 -Force | Out-Null
Import-PackageProvider NuGet -Force | Out-Null
Set-PSRepository -Name PSGallery -InstallationPolicy Trusted | Out-Null
Install-Module InvokeBuild -RequiredVersion 3.2.1 -Scope CurrentUser -Force | Out-Null
Copy link
Contributor

Choose a reason for hiding this comment

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

So how did this break if we were specifying a required version of InvokeBuild? Updates to InvokeBuild shouldn't have broken our build.

Copy link
Member Author

Choose a reason for hiding this comment

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

It actually broke Travis who didn't have a required version.

Copy link
Contributor

@rkeithhill rkeithhill Jan 2, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

But that parameter is for Install-Module. Oh, so maybe Travis is using a version of PowerShellGet that doesn't have that parameter? Er, that doesn't make sense because this next line uses this parameter:

Install-Module platyPS -RequiredVersion 0.7.6 -Scope CurrentUser -Force | Out-Null

I think it is OK to update occassionally to the latest version of InvokeBuild. I just don't know that we want to do that everytime we build. In fact, the -RequiredVersion parameter is meant to prevent the very error we encountered - "new release of InvokeBuild breaks our build".

OK, I think I see what is going on here. It is the travis build that is breaking, right? That build doesn't use appveyor.yml. It uses .travis.yml which runs build\travis.ps1. And that script has this line in it:

Install-Module InvokeBuild -Scope CurrentUser -Force

It seems to me that we need to add -RequiredVersion to the above command in travis.ps1 and bump it to 5.0.0 and put the parameter back in appveyor.yml and bump it up to 5.0.0 as well.

And add the -RequiredVersion to the install of platyPS in travis.ps1 as well.

Copy link
Member Author

Choose a reason for hiding this comment

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

Instead of -RequiredVersion I've specified -MaximumVersion so that we can still get patch updates without the harsh breaking changes of a top level version number (v5 -> v6 for example).

Let me know if you feel strongly otherwise 😄

Copy link
Collaborator

Choose a reason for hiding this comment

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

At least for platyPS we should stick with -RequiredVersion because they haven't hit 1.0 yet. I don't recall them having any big breaking changes, but there isn't anything stating they won't afaict

Copy link
Member Author

Choose a reason for hiding this comment

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

@SeeminglyScience I've updated it everywhere to require the 0.9.0 version of the package. This is the version that Travis was using. For Windows, this means it has been updated from 0.7.6 to 0.9.0. Yay consistency!

Install-Module platyPS -RequiredVersion 0.7.6 -Scope CurrentUser -Force | Out-Null
Install-Module InvokeBuild -MaximumVersion 5.1.0 -Scope CurrentUser -Force | Out-Null
Install-Module platyPS -RequiredVersion 0.9.0 -Scope CurrentUser -Force | Out-Null

build_script:
- ps: Invoke-Build -Configuration Release
Expand Down
4 changes: 2 additions & 2 deletions scripts/travis.ps1
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@


# Install InvokeBuild
Install-Module InvokeBuild -Scope CurrentUser -Force
Install-Module PlatyPS -Scope CurrentUser -Force
Install-Module InvokeBuild -MaximumVersion 5.1.0 -Scope CurrentUser -Force
Install-Module PlatyPS -RequiredVersion 0.9.0 -Scope CurrentUser -Force


# Build the code and perform tests
Expand Down