test-case: check-sof-logger to fix issue of log file #161
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
xiulipan
reviewed
Apr 2, 2020
xiulipan
previously requested changes
Apr 2, 2020
Contributor
xiulipan
left a comment
xiulipan
left a comment
There was a problem hiding this comment.
The new function is good, but can you add more inline comments and refine you commit message.
aiChaoSONG
suggested changes
Apr 2, 2020
aiChaoSONG
left a comment
aiChaoSONG
left a comment
There was a problem hiding this comment.
Code changes are good, now the commit message.
As you did so much in this one commit, the Commit message title -> refine check-sof-logger.sh test case
as this is not done, let's discuss commit message body later
Contributor
Author
|
@xiulipan @aiChaoSONG @fredoh9 please help to review |
aiChaoSONG
suggested changes
Apr 2, 2020
aiChaoSONG
left a comment
aiChaoSONG
left a comment
There was a problem hiding this comment.
commit message body:
Previously, if sof-logger only catchs the firmware log header, we will
fail the test case. Obviously, this is an overaction. So this patch relaxes
the firmware log check rule by allowing the log size less than 4K.
As this test case focuses on sof-logger function check, if firmware
error is found in the log, we just print out the error without failing
the test case.
Previously, if sof-logger only catchs the firmware log header, we will fail the test case. Obviously, this is an overaction. So this patch relaxes the firmware log check rule by allowing the log size less than 4K. As this test case focuses on sof-logger function check, if firmware error is found in the log, we just print out the error without failing the test case. Signed-off-by: Wu, BinX <binx.wu@intel.com>
aiChaoSONG
approved these changes
Apr 2, 2020
libinyang
reviewed
Apr 3, 2020
marc-hb
added a commit
to marc-hb/sof-test
that referenced
this pull request
Apr 27, 2021
Probably the main change is fixing the huge etrace test gaps thesofproject#321 and thesofproject/sof#3281 Also fixes DMA trace gaps thesofproject#297 and thesofproject#298 I initial tried to preserve some of the existing code but it was just too bad. PR thesofproject#161 / commit 9136776 seemed especially bad: - It tried to ignore a specific `ll drift` error but instead it filtered out almost every log statement out of... stderr, that does not have show log statements!! (Just for the record this `ll drift` error has been downgraded to warning now, see thesofproject/sof#2686 and thesofproject/sof#3854) - That same commit also added code that merely starts the DMA trace with "there is an error below" (without failing the test) but that's eclipsed by the entire log that follows. Later, the firmware started printing ERROR every single time when the ERROR FW ABI prefix was introduced yet no one ever noticed which proves how useless this prefix is was. So remove this DMA trace prefix as the purpose of this test is - as clearly stated in thesofproject#167 - not to find firmware errors but errors with the sof-logger itself (even though we never had anything looking at firmware errors so far) Don't grep for "error" on stderr: anything on stderr is a logger failure (not a firmware failure). Don't require whitespace before the TIMESTAMP header. Add set -e. Use shell functions. Signed-off-by: Marc Herbert <marc.herbert@intel.com>
marc-hb
added a commit
to marc-hb/sof-test
that referenced
this pull request
Apr 27, 2021
Probably the main change is fixing the huge etrace test gaps thesofproject#321 and thesofproject/sof#3281 Also fixes DMA trace gaps thesofproject#297 and thesofproject#298 I initial tried to preserve some of the existing code but it was just too bad. PR thesofproject#161 / commit 7274f49 seemed especially bad: - It tried to ignore a specific `ll drift` error but instead it filtered out almost every log statement out of... stderr, that does not have show log statements!! (Just for the record this `ll drift` error has been downgraded to warning now, see thesofproject/sof#2686 and thesofproject/sof#3854) - That same commit also added code that merely starts the DMA trace with "there is an error below" (without failing the test) but that's eclipsed by the entire log that follows. Later, the firmware started printing ERROR every single time when the ERROR FW ABI prefix was introduced yet no one ever noticed which proves how useless this prefix is was. So remove this DMA trace prefix as the purpose of this test is - as clearly stated in thesofproject#167 - not to find firmware errors but errors with the sof-logger itself (even though we never had anything looking at firmware errors so far) Don't grep for "error" on stderr: anything on stderr is a logger failure (not a firmware failure). Don't require whitespace before the TIMESTAMP header. Add set -e. Use shell functions. Signed-off-by: Marc Herbert <marc.herbert@intel.com>
marc-hb
added a commit
to marc-hb/sof-test
that referenced
this pull request
Apr 27, 2021
Probably the main change is fixing the huge etrace test gaps thesofproject#321 and thesofproject/sof#3281 Also fixes DMA trace gaps thesofproject#297 and thesofproject#298 I initial tried to preserve some of the existing code but it was just too bad. PR thesofproject#161 / commit 7274f49 seemed especially bad: - It tried to ignore a specific `ll drift` error but instead it filtered out almost every log statement out of... stderr, that does not have show log statements!! (Just for the record this `ll drift` error has been downgraded to warning now, see thesofproject/sof#2686 and thesofproject/sof#3854) - That same commit also added code that merely starts the DMA trace with "there is an error below" (without failing the test) but that's eclipsed by the entire log that follows. Later, the firmware started printing ERROR every single time when the ERROR FW ABI prefix was introduced yet no one ever noticed which proves how useless this prefix is was. So remove this DMA trace prefix as the purpose of this test is - as clearly stated in thesofproject#167 - not to find firmware errors but errors with the sof-logger itself (even though we never had anything looking at firmware errors so far) Don't grep for "error" on stderr: anything on stderr is a logger failure (not a firmware failure). Don't require whitespace before the TIMESTAMP header. Add set -e. Use shell functions. Signed-off-by: Marc Herbert <marc.herbert@intel.com>
marc-hb
added a commit
to marc-hb/sof-test
that referenced
this pull request
Apr 28, 2021
Probably the main change is fixing the huge etrace test gaps thesofproject#321 and thesofproject/sof#3281 Also fixes DMA trace gaps thesofproject#297 and thesofproject#298 I initial tried to preserve some of the existing code but it was just too bad. PR thesofproject#161 / commit 7274f49 seemed especially bad: - It tried to ignore a specific `ll drift` error but instead it filtered out almost every log statement out of... stderr, that does not have show log statements!! (Just for the record this `ll drift` error has been downgraded to warning now, see thesofproject/sof#2686 and thesofproject/sof#3854) - That same commit also added code that merely starts the DMA trace with "there is an error below" (without failing the test) but that's eclipsed by the entire log that follows. Later, the firmware started printing ERROR every single time when the ERROR FW ABI prefix was introduced yet no one ever noticed which proves how useless this prefix is was. So remove this DMA trace prefix as the purpose of this test is - as clearly stated in thesofproject#167 - not to find firmware errors but errors with the sof-logger itself (even though we never had anything looking at firmware errors so far) Don't grep for "error" on stderr: anything on stderr is a logger failure (not a firmware failure). Don't require whitespace before the TIMESTAMP header. Add set -e. Use shell functions. Signed-off-by: Marc Herbert <marc.herbert@intel.com>
marc-hb
added a commit
to marc-hb/sof-test
that referenced
this pull request
Jun 9, 2021
Probably the main change is fixing the huge etrace test gaps thesofproject#321 and thesofproject/sof#3281 Also fixes DMA trace gaps thesofproject#297 and thesofproject#298 I initial tried to preserve some of the existing code but it was just too bad. PR thesofproject#161 / commit 7274f49 seemed especially bad: - It tried to ignore a specific `ll drift` error but instead it filtered out almost every log statement out of... stderr, that does not have show log statements!! (Just for the record this `ll drift` error has been downgraded to warning now, see thesofproject/sof#2686 and thesofproject/sof#3854) - That same commit also added code that merely starts the DMA trace with "there is an error below" (without failing the test) but that's eclipsed by the entire log that follows. Later, the firmware started printing ERROR every single time when the ERROR FW ABI prefix was introduced yet no one ever noticed which proves how useless this prefix is was. So remove this DMA trace prefix as the purpose of this test is - as clearly stated in thesofproject#167 - not to find firmware errors but errors with the sof-logger itself (even though we never had anything looking at firmware errors so far) Don't grep for "error" on stderr: anything on stderr is a logger failure (not a firmware failure). Don't require whitespace before the TIMESTAMP header. Add set -e. Use shell functions. Signed-off-by: Marc Herbert <marc.herbert@intel.com>
marc-hb
added a commit
that referenced
this pull request
Jun 10, 2021
Probably the main change is fixing the huge etrace test gaps #321 and thesofproject/sof#3281 Also fixes DMA trace gaps #297 and #298 I initial tried to preserve some of the existing code but it was just too bad. PR #161 / commit 7274f49 seemed especially bad: - It tried to ignore a specific `ll drift` error but instead it filtered out almost every log statement out of... stderr, that does not have show log statements!! (Just for the record this `ll drift` error has been downgraded to warning now, see thesofproject/sof#2686 and thesofproject/sof#3854) - That same commit also added code that merely starts the DMA trace with "there is an error below" (without failing the test) but that's eclipsed by the entire log that follows. Later, the firmware started printing ERROR every single time when the ERROR FW ABI prefix was introduced yet no one ever noticed which proves how useless this prefix is was. So remove this DMA trace prefix as the purpose of this test is - as clearly stated in #167 - not to find firmware errors but errors with the sof-logger itself (even though we never had anything looking at firmware errors so far) Don't grep for "error" on stderr: anything on stderr is a logger failure (not a firmware failure). Don't require whitespace before the TIMESTAMP header. Add set -e. Use shell functions. Signed-off-by: Marc Herbert <marc.herbert@intel.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
6 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
HEAD which size is 4K by 'du' command
dump really error of sof
Signed-off-by: Wu, BinX binx.wu@intel.com