GitHub PullRequestReviewCommentEvent at 2014-02-26 10:26:23
Repository: core
Actor: kacfengine
Payload: {"comment":{"url":"https://api.github.com/repos/cfengine/core/pulls/comments/10074590","id":10074590,"diff_hunk":"@@ -294,10 +298,34 @@ export CFENGINE_TEST_OVERRIDE_WORKDIR TEMP CFENGINE_TEST_OVERRIDE_EXTENSION_LIBR\n     TEST_END_TIME=$(unix_seconds)\n     echo \"              \"time=\\\"$(($TEST_END_TIME - $TEST_START_TIME)) seconds\\\"\\> >> \"$XMLTMP\"\n \n-    if [ -z \"$EXPECTED_CRASH\" ]; then\n-      ESCAPED_TEST=\"$(echo \"$TEST\" | sed -e 's/\\./\\\\./g')\"\n-      if [ $RETVAL -eq 0 ] && echo \"$OUT\" | grep -q -e \"R: .*$ESCAPED_TEST Pass\"; then\n+    RESULT_MSG=\n+    if [ -z \"$EXPECTED_CRASH\" ] && [ $RETVAL -ne 0 ]; then\n+      RESULT=FAIL\n+    elif [ -z \"$EXPECTED_CRASH\" ]; then\n+      # Some states are output by default.cf.sub, therefore check for both TEST\n+      # prefix and default.cf.sub prefix.\n+      ESCAPED_TEST=\"$(echo \"\\($TEST\\|default.cf.sub\\)\" | sed -e 's/\\./\\\\./g')\"\n+      if echo \"$OUT\" | grep -q -e \"R: .*$ESCAPED_TEST XFAIL\"; then\n+        # Check for other test case states than fail. Should not happen.\n+        if echo \"$OUT\" | grep -e \"R: .*$ESCAPED_TEST \" | grep -v -q -e \"R: .*$ESCAPED_TEST X\\?FAIL\"; then\n+          RESULT=FAIL\n+          RESULT_MSG=\"FAIL (Tried to suppress nonexistent failure)\"","path":"tests/acceptance/testall","position":40,"original_position":40,"commit_id":"2e7aebf4bd276d272d9b806fec5a70f9710865cf","original_commit_id":"2e7aebf4bd276d272d9b806fec5a70f9710865cf","user":{"login":"kacfengine","id":4037094,"avatar_url":"https://gravatar.com/avatar/051c8986741264f2671890948d4f4d30?d=https%3A%2F%2Fidenticons.github.com%2F196c8bcd77dbf21d0302985918a4aeae.png&r=x","gravatar_id":"051c8986741264f2671890948d4f4d30","url":"https://api.github.com/users/kacfengine","html_url":"https://github.com/kacfengine","followers_url":"https://api.github.com/users/kacfengine/followers","following_url":"https://api.github.com/users/kacfengine/following{/other_user}","gists_url":"https://api.github.com/users/kacfengine/gists{/gist_id}","starred_url":"https://api.github.com/users/kacfengine/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/kacfengine/subscriptions","organizations_url":"https://api.github.com/users/kacfengine/orgs","repos_url":"https://api.github.com/users/kacfengine/repos","events_url":"https://api.github.com/users/kacfengine/events{/privacy}","received_events_url":"https://api.github.com/users/kacfengine/received_events","type":"User","site_admin":false},"body":"Ah, there is an extra snag here: Because CFEngine iterates over the promise in passes, each new pass may yield new results. Therefore it is quite common for tests to output both FAIL and Pass, because the conditions didn't satisfy the first time around, but the latter always trumps the former. I suppose a comment about this somewhere would be helpful.","created_at":"2014-02-26T10:26:23Z","updated_at":"2014-02-26T10:26:23Z","html_url":"https://github.com/cfengine/core/pull/1476#discussion_r10074590","pull_request_url":"https://api.github.com/repos/cfengine/core/pulls/1476","_links":{"self":{"href":"https://api.github.com/repos/cfengine/core/pulls/comments/10074590"},"html":{"href":"https://github.com/cfengine/core/pull/1476#discussion_r10074590"},"pull_request":{"href":"https://api.github.com/repos/cfengine/core/pulls/1476"}}}}