Skip to content

hijack.sh: switch first attempt to stop the logger from -INT to -TERM - #382

Merged
xiulipan merged 1 commit into
thesofproject:masterfrom
marc-hb:term-logger
Sep 23, 2020
Merged

xiulipan merged 1 commit into
thesofproject:masterfrom
marc-hb:term-logger

Conversation

@marc-hb

@marc-hb marc-hb commented Sep 18, 2020

Copy link
Copy Markdown
Collaborator

Commit 61271a3 ("logger: stop discarding all error messages and fix
the cleanup code") made two mistakes:

  1. It used the -INT signal which doesn't always work. This broke most
    tests.
  2. It then misused exit_failure=2 where 2 means Not Applicable. This
    turned most broken tests to green.

Rushed commit a2669ff ("hijack.sh: don't fail yet when sof-logger
resists kill -INT") then downgraded all this to warning to get the tests
passing again. However it didn't deal with the real issue: stopping the
logger on the first try. Switching to TERM does it and will hopefully
close https://github.com/thesofproject/sof/issues/3433

Signed-off-by: Marc Herbert marc.herbert@intel.com

Commit 61271a3 ("logger: stop discarding all error messages and fix
the cleanup code") made two mistakes:

1. It used the -INT signal which doesn't always work. This broke most
   tests.
2. It then misused exit_failure=2 where 2 means Not Applicable. This
  turned most broken tests to green.

Rushed commit a2669ff ("hijack.sh: don't fail yet when sof-logger
resists kill -INT") then downgraded all this to warning to get the tests
passing again. However it didn't deal with the real issue: stopping the
logger on the first try. Switching to TERM does it and will hopefully
close https://github.com/thesofproject/sof/issues/3433

Signed-off-by: Marc Herbert <marc.herbert@intel.com>
@marc-hb
marc-hb marked this pull request as ready for review September 18, 2020 04:27
@marc-hb
marc-hb requested a review from a team as a code owner September 18, 2020 04:27
@marc-hb
marc-hb marked this pull request as draft September 18, 2020 04:28
@marc-hb

marc-hb commented Sep 18, 2020 •

Copy link
Copy Markdown
Collaborator Author

CI results look good so this PR looks ready to me but I'm keeping this as a draft for a few hours to keep a "safe distance" with previous workaround #381 that was just merged.. If they are too close to each other it will be very difficult to determine which build used which sof-test version (there is no sof-test version information in test results yet)

@marc-hb
marc-hb marked this pull request as ready for review September 18, 2020 16:59

@xiulipan xiulipan left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM.

@xiulipan
xiulipan merged commit 0af6a25 into thesofproject:master Sep 23, 2020
@marc-hb
marc-hb deleted the term-logger branch September 28, 2020 14:42
@marc-hb marc-hb added the area:logs Log and results collection, storage, etc. label Jul 3, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:logs Log and results collection, storage, etc.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants