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

feat: JobLoggerFactory #1496

Merged
merged 1 commit into from
Dec 9, 2022
Merged

Conversation

ChristopherHX
Copy link
Contributor

@ChristopherHX ChristopherHX commented Dec 8, 2022

  • Remove overriding io.Stdout in TestMaskValues to prevent deadlock in GitHub Actions
  • Print log of TestMaskValues in Test

captureOutput is unsafe and causes a deadlock in #1472

I tested merging this change into #1472 and it unblocks the failing test.

Remove overriding io.Stdout in TestMaskValues to prevent deadlock in GitHub Actions
@ChristopherHX ChristopherHX requested a review from a team as a code owner December 8, 2022 21:37
@codecov
Copy link

codecov bot commented Dec 8, 2022

Codecov Report

Merging #1496 (5a5c1a5) into master (4f8da0a) will increase coverage by 3.72%.
The diff coverage is 67.61%.

@@            Coverage Diff             @@
##           master    #1496      +/-   ##
==========================================
+ Coverage   57.50%   61.23%   +3.72%     
==========================================
  Files          32       45      +13     
  Lines        4594     6960    +2366     
==========================================
+ Hits         2642     4262    +1620     
- Misses       1729     2397     +668     
- Partials      223      301      +78     
Impacted Files Coverage Δ
pkg/common/file.go 0.00% <0.00%> (ø)
pkg/container/docker_logger.go 52.08% <ø> (ø)
pkg/container/host_environment.go 0.00% <0.00%> (ø)
pkg/container/parse_env_file.go 0.00% <0.00%> (ø)
pkg/container/util.go 0.00% <0.00%> (ø)
pkg/model/action.go 0.00% <0.00%> (ø)
pkg/model/step_result.go 0.00% <ø> (ø)
pkg/container/docker_run.go 13.58% <11.48%> (+8.04%) ⬆️
...ontainer/linux_container_environment_extensions.go 24.32% <24.32%> (ø)
pkg/model/workflow.go 46.43% <25.25%> (-4.48%) ⬇️
... and 38 more

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@KnisterPeter
Copy link
Member

Looks fine. Strange that it didn't deadlock earlier, as that code is in master for a long time already.

@mergify mergify bot merged commit 7d99512 into nektos:master Dec 9, 2022
@ChristopherHX
Copy link
Contributor Author

Also strange that it only deadlocked in GitHub Actions Hosted Runner and I couldn't even reproduce it in GitHub Codespacs both on linux.

@ChristopherHX ChristopherHX deleted the feat-jobloggerfactory branch December 9, 2022 16:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants