alfa icon indicating copy to clipboard operation
alfa copied to clipboard

`Sequence` are not as transparent as they should

Open Jym77 opened this issue 6 months ago • 0 comments

While working on alfa-test-utils, I've noticed a weird case where forcing a sequence or not changes the results of a function, which should not happen due to referencial transparency. This seems to be linked to use of .groupBy as before using it nothing happened. Maybe also linked to hashability of string Enum.

  export async function run(
    page: Page,
    options: Options = {}
  ): Promise<Result> {
    const rulesToRun =
      options.rules?.override ?? false
        ? options.rules?.custom ?? []
        : filter(Rules.allRules, options.rules).concat(
            options.rules?.custom ?? []
          );

    const outcomes = Sequence.from(
      await alfaAudit.of(page, rulesToRun).evaluate()
    );
    const ResultAggregates = aggregates(outcomes);

    return {
      alfaVersion,
      page,
      outcomes: filter(outcomes, options.outcomes),
      ResultAggregates,
    };
  }

export function aggregates(
    outcomes: Sequence<alfaOutcome>
  ): Iterable<RuleAggregate> {
    return (
      outcomes
        // Group by rule URI
        .groupBy((outcome) => outcome.rule.uri)
        // For each rule, group by outcome
        .map((ruleOutcomes) =>
          ruleOutcomes.groupBy((outcome) => outcome.outcome)
        )
        // Count the size of each group and build the aggregates
        .map((groups, uri) => ({
          RuleId: uri,
          Failed: groups.get(Outcome.Value.Failed).getOrElse(Sequence.empty)
            .size,
          Passed: groups.get(Outcome.Value.Passed).getOrElse(Sequence.empty)
            .size,
          CantTell: groups.get(Outcome.Value.CantTell).getOrElse(Sequence.empty)
            .size,
        }))
        .values()
    );
  }

Inlining the computation of ResultAggregates discards all but the first outcomes from outcomes. Forcing the evaluation of the sequence (e.g. with a dummy [...outcomes], or … .toJSON() (which makes investigations tricky)) restores every thing. None of these should be impacting the result.

Given that this is the first time we notice this, I'm letting it as is for now.

Jym77 avatar Aug 05 '24 07:08 Jym77