-
Notifications
You must be signed in to change notification settings - Fork 7.7k
Jameswtruher/travisdaily2 #2842
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
Jameswtruher/travisdaily2 #2842
Conversation
Travis is still complaining that the log file is too big, eliminating progress is another step to reducing output. It's also not needed for our CI environment.
Maybe progress should be opt-in instead of opt-out. In other words, disable globally, turn on in the progress tests. |
also fix up trailing whitespace
I thought about disabling progress universally but thought it would be better for each test to control this behavior. My concern is reviewing/rewriting existing tests which may be relying on progress in ways that I don't know. |
Which seems more likely?
With code coverage data, I feel comfortable that 1 won't be a big problem. Also, when progress doesn't work correctly, because it's UI, folks tend to notice and complain. I think 2 will be less hassle in the long run. |
oh, we know we'll be adding a number of progress tests - right now we are only doing the minimum amount of progress tests. As soon as we have better support on non-windows we'll be doing even more. I don't agree that #2 is less hassle |
also, i don't know that this will even solve the problem with Travis-CI, but I have no other way to test it - i would much rather make targeted changes to determine if it will solve the daily build in Travis-CI issue than whole scale changing the way our tests work |
Isn't this really just a bug in PowerShell? Shouldn't we be detecting that we're not running interactively and not show progress? |
That I don't know - I recall that we had difficulties with running (historical) tests because some host APIs would throw when in that mode. This code base is different, of course, but I still would rather avoid tracking down extraneous issues when all I'm trying to do is find the right incantation to get daily builds in Travis-CI to work. |
Current solution solved the Travis full log problem. It is very good! As regards global disable progress bar. On the one hand testing should be performed with the default values as users get them. On the other hand any default parameter changes must not lead to the collapse of the tests, nor to the collapse of the real-world scripts. |
can we please merge this now? |
Please remove the formatting changes from the code changes, it unnecessarily makes it harder to see the logical code changes. We have a different PR to remove trailing spaces, so we don't need to do that file by file. |
@@ -8,6 +8,8 @@ | |||
# (when calling get-help -online) might not matched the one in the csv file. | |||
|
|||
BeforeAll { | |||
$SavedProgressPreference = $ProgressPreference |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this is a Pester issues, but this pattern can result in the preference not being set back to the original setting. What should we do about this:
- File an issue with Pester
- Use another pattern
- Live with the issue
- Some other option
Example Code:
PS C:\Users\tplunk> Describe 'before all after all' {
BeforeAll{
Write-Verbose "in BeforeAll" -Verbose
}
it "it" {
Start-Sleep -Seconds 2
}
AfterAll {
Write-Verbose "in AfterAll" -Verbose
}
}
Example hitting ctl-c during it:
Describing before all after all
VERBOSE: in BeforeAll
VERBOSE: in it
Note: After all was not run
Example of full run
Describing before all after all
VERBOSE: in BeforeAll
VERBOSE: in it
[+] it 2.13s
VERBOSE: in AfterAll
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll file an issue with the Pester, this isn't likely to happen in a run (as there's no one to press CRL-C) - This problem is general in pester over all.
I've opened pester/Pester#655
@@ -18,40 +18,44 @@ $InternalSource = 'OneGetTestSource' | |||
|
|||
|
|||
Describe "PackageManagement Acceptance Test" -Tags "Feature" { | |||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In the future, please avoid making formatting changes in the same PR as code change per the Coding Conventions.
continued attempts to reduce output size so we can get a daily build in travis-ci
this sets progress preference on the help and package management tests to SilentlyContinue to reduce output. This is done for all test execution, including dev and CI