Fix allow_failure when combined with rules (#1283)

* test: Check that `allow_failure` does not get overwritten

Currently `allow_failure` is overwritten by `rules` regardless of
whether or not `rules` has an `allow_failure` setting itself.

* fix: Do not overwrite `allow_failure` from `rules` unless set

Author: PigeonF

src/job.ts

@@ -194,7 +194,7 @@ export class Job {
 
         // Set {when, allowFailure} based on rules result
         if (this.rules) {
-            const ruleResult = Utils.getRulesResult({cwd, rules: this.rules, variables: this._variables}, this.gitData, this.when);
+            const ruleResult = Utils.getRulesResult({cwd, rules: this.rules, variables: this._variables}, this.gitData, this.when, this.allowFailure);
             this.when = ruleResult.when;
             this.allowFailure = ruleResult.allowFailure;
             this._variables = {...globalVariables, ...jobVariables, ...ruleResult.variables, ...matrixVariables, ...predefinedVariables, ...envMatchedVariables, ...argvVariables};

src/utils.ts

@@ -180,11 +180,11 @@ export class Utils {
         return envMatchedVariables;
     }
 
-    static getRulesResult (opt: RuleResultOpt, gitData: GitData, jobWhen: string = "on_success"): {when: string; allowFailure: boolean; variables?: {[name: string]: string}} {
+    static getRulesResult (opt: RuleResultOpt, gitData: GitData, jobWhen: string = "on_success", jobAllowFailure: boolean | {exit_codes: number | number[]} = false): {when: string; allowFailure: boolean | {exit_codes: number | number[]}; variables?: {[name: string]: string}} {
         let when = "never";
 
         // optional manual jobs allowFailure defaults to true https://docs.gitlab.com/ee/ci/jobs/job_control.html#types-of-manual-jobs
-        let allowFailure = jobWhen === "manual";
+        let allowFailure = jobWhen === "manual" ? true : jobAllowFailure;
         let ruleVariable: {[name: string]: string} | undefined;
 
         for (const rule of opt.rules) {

tests/test-cases/script-failures/.gitlab-ci.yml

@@ -62,9 +62,18 @@ exit_code[number[]] not allowed:
 
 rules:allow_failure precedence:
   image: nginx:alpine
-  allow_failure: false
+  allow_failure:
+    exit_codes: 137
   rules:
     - if: $CI_DEFAULT_BRANCH == "main"
-      allow_failure: true
+      allow_failure: false
   script:
-    - exit 1
+    - exit 137
+
+rules:without allow_failure:
+  allow_failure:
+    exit_codes: 137
+  rules:
+    - when: always
+  script:
+    - exit 137

tests/test-cases/script-failures/integration.script-failures.test.ts

@@ -141,6 +141,18 @@ test("script-failures <rules:allow_failure precedence>", async () => {
     }, writeStreams);
 
     expect(writeStreams.stdoutLines.join("\n")).toContain(
-        chalk`{black.bgYellowBright  WARN } {blueBright rules:allow_failure precedence}`,
+        chalk`{black.bgRed  FAIL } {blueBright rules:allow_failure precedence}`,
+    );
+});
+
+test("script-failures <rules:without allow_failure>", async () => {
+    const writeStreams = new WriteStreamsMock();
+    await handler({
+        cwd: "tests/test-cases/script-failures",
+        job: ["rules:without allow_failure"],
+    }, writeStreams);
+
+    expect(writeStreams.stdoutLines.join("\n")).toContain(
+        chalk`{black.bgYellowBright  WARN } {blueBright rules:without allow_failure}`,
     );
 });