Repository navigation
CFE-3041: Made testall fail regular acceptance tests which log errors - #6405
aleksandrychev wants to merge 15 commits into
Conversation
Tests that print Pass message but have logs errors will fail with `FAIL (Error printed)` message. Tests which are expected to log errors should use `.error.cf` or `.error.pass.cf` suffix. see: `tests/acceptance/README` Ticket: CFE-3041 Signed-off-by: Ihor Aleksandrychiev <ihor.aleksandrychiev@northern.tech>
|
@cf-bottom jenkins please |
|
Sure, I triggered a build: Jenkins: https://ci.cfengine.com/job/pr-pipeline/14799/ Packages: http://buildcache.cfengine.com/packages/testing-pr/jenkins-pr-pipeline-14799/ |
olehermanse
left a comment
There was a problem hiding this comment.
This cannot be done on its own - there are hundreds of tests that need to be reviewed and adjusted after this change in testall.
I see, there are quite a lot of them. So, we need to identify which ones are expected and should be renamed, and also find the ones that shouldn't be there. As a first step, I'll create a table listing each failure and the errors printed. |
Acceptance tests failing with "Error printed"
|
|
at least half of errors can be fixed easily with a fileexists guard. the error comes from the vars are evaluated early (during policy validation and pre-evaluation), before it can be fixed by adding a fileexists guard. |
These tests read files created by their own init bundle, but pre-evaluation calls the read functions before the files exist, logging "Could not examine file" errors which now fail regular tests. Ticket: CFE-3041 Signed-off-by: Ihor Aleksandrychiev <ihor.aleksandrychiev@northern.tech>
Errors printed by the command were reported on lines starting with "error:", so testall mistook them for errors of the test itself. Ticket: CFE-3041 Signed-off-by: Ihor Aleksandrychiev <ihor.aleksandrychiev@northern.tech>
These tests read files and directories created by their own policy, but pre-evaluation calls the functions before they exist, logging errors which now fail regular tests. Ticket: CFE-3041 Signed-off-by: Ihor Aleksandrychiev <ihor.aleksandrychiev@northern.tech>
The agent loads def.json from the directory of the input policy file, so tests in 00_basics/ tried to read this directory as their augments file and logged "Could not open JSON file" errors. Ticket: CFE-3041 Signed-off-by: Ihor Aleksandrychiev <ihor.aleksandrychiev@northern.tech>
The test used the 'immediate' body from plucked.sub.cf before it was loaded during pre-evaluation, so the promise is now skipped until the test framework is loaded. Ticket: CFE-3041 Signed-off-by: Ihor Aleksandrychiev <ihor.aleksandrychiev@northern.tech>
The test referred to a promise module which never existed and did not run its init and cleanup bundles. Ticket: CFE-3041 Signed-off-by: Ihor Aleksandrychiev <ihor.aleksandrychiev@northern.tech>
The check bundle counted lines in files which do not exist until the test bundle has run, but the agent evaluates the functions of all promises before any bundle runs. Ticket: CFE-3041 Signed-off-by: Ihor Aleksandrychiev <ihor.aleksandrychiev@northern.tech>
wrong-bundle-type.cf printed the sub-agent output in its DEBUG report, so the sub-agent errors were mistaken for errors of the test, and ifelse.cf used invalid class expressions as false conditions. Ticket: CFE-3041 Signed-off-by: Ihor Aleksandrychiev <ihor.aleksandrychiev@northern.tech>
These tests intentionally exercise error paths, the reason is documented in the header of each test. Ticket: CFE-3041 Signed-off-by: Ihor Aleksandrychiev <ihor.aleksandrychiev@northern.tech>
Signed-off-by: Ihor Aleksandrychiev <ihor.aleksandrychiev@northern.tech>
There was a problem hiding this comment.
@aleksandrychev The commit for Added fileexists() guards to file reads in acceptance tests is good. I think we could take it separately. Will continue looking into the other commits.
Looked through the rest and they all seem ok. Logged CFE-4752 which i found while reviewing.
| "lefti" int => "$(init.pair[1])"; | ||
| "rights" string => "$(init.pair[2])"; | ||
| "rightr" real => "$(init.pair[2])"; | ||
| "rightr" real => "$(init.pair[2])", |
There was a problem hiding this comment.
This one is a bit odd. Maybe the question is why does only real vars log an error fail to parse real number when it encounters an empty string. Seems that we should align the int and real logging behavior here. I created CFE-4752 for that.
| bundle common framework | ||
| { | ||
| classes: | ||
| "framework_loaded" expression => isvariable("G.testdir"); |
There was a problem hiding this comment.
Strange. I guess this can affect all cases where a body is used (on classes or vars promises) in the entry file or in some bundle before reaching the body in pre-eval.
| bundle common framework | ||
| { | ||
| classes: | ||
| "framework_loaded" expression => isvariable("G.testdir"); |
There was a problem hiding this comment.
Strange. I guess this can affect all cases where a body is used (on classes or vars promises) in the entry file or in some bundle before reaching the body in pre-eval.
|
@olehermanse your requested change blocks PR |
|
@cf-bottom jenkins please |
|
Alright, I triggered a build: Jenkins: https://ci.cfengine.com/job/pr-pipeline/14855/ Packages: http://buildcache.cfengine.com/packages/testing-pr/jenkins-pr-pipeline-14855/ |
|
@aleksandrychev please look at why Ubuntu failed. |
I will look, maybe this is environment issue or platform-specific, as it passes in GH runner (ubuntu 24) and also passed in docker container (same as GH runner uses) |
Ticket: CFE-3041 Signed-off-by: Nick Anderson <nick@cmdln.org>
Ticket: CFE-3041 Signed-off-by: Nick Anderson <nick@cmdln.org>
Ticket: CFE-3041 Signed-off-by: Nick Anderson <nick@cmdln.org>
|
@cf-bottom jenkins please |
|
Sure, I triggered a build: Jenkins: https://ci.cfengine.com/job/pr-pipeline/14858/ Packages: http://buildcache.cfengine.com/packages/testing-pr/jenkins-pr-pipeline-14858/ |
|
@nickanderson thanks for help here, now only diff_reporting_large_file.cf fails, looking at it |
|
@cf-bottom jenkins please |
|
Alright, I triggered a build: Jenkins: https://ci.cfengine.com/job/pr-pipeline/14860/ Packages: http://buildcache.cfengine.com/packages/testing-pr/jenkins-pr-pipeline-14860/ |
|
@cf-bottom jenkins please |
|
Alright, I triggered a build: Jenkins: https://ci.cfengine.com/job/pr-pipeline/14861/ Packages: http://buildcache.cfengine.com/packages/testing-pr/jenkins-pr-pipeline-14861/ |
cb66e02 to
b1cc006
Compare
Ticket: CFE-3041
b1cc006 to
b7dde0c
Compare
|
@cf-bottom jenkins please |
|
Sure, I triggered a build: Jenkins: https://ci.cfengine.com/job/pr-pipeline/14863/ Packages: http://buildcache.cfengine.com/packages/testing-pr/jenkins-pr-pipeline-14863/ |
Tests that print Pass message but have logs errors will fail with
FAIL (Error printed)message. Tests which are expected to log errors should use.error.cfor.error.pass.cfsuffix. see:tests/acceptance/READMEtogether:
Ticket: CFE-3041