feat: dedicated reporter command

[ruocco-l]

Closes #119

Adds a notify command that delivers test and release notifications through a pluggable notifier (Slack for now), decoupled from the commands that produce them.

• create-test-report and release don't send notifications or build any message text themselves — they each write a typed NotifyDocument describing what happened, always to stdout (or stderr on --dry-run, so it stays inspectable), and optionally to a file via --output. TODO These two commands will probably be reworked but this was the easiest way to make the diff on this PR at a minimum.
• notify reads that document — from --input or piped in on stdin — and hands it to the chosen notifier's format() method, which turns the structured data into that notifier's message and decides whether there's anything worth sending at all (e.g. a test-report with zero failures sends nothing). Each notifier is a class (assertConfig/format/send), so adding a new one is a matter of implementing that interface, not branching inside notify. Each new notifier would be responsible to create their own formatting for each document type.
• A release can be reported for two audiences — developers (full commit list + changed files) and the public (changelog entries only) — from the same document; notify --view <dev|public> picks which prose to render, so callers run notify twice (once per view) against the one document release produced.
• Each test failure gets a stable id (built from the actor and the full test name) so the same failure can be recognized across separate runs, e.g. for tracking flaky tests over time.
• Delivery settings live in apify-test-tools.config.json, under notifiers.<name>. Every notifier has a targets map — one entry per document type it can send (release-report entries are further keyed by view) — and only needs the specific entry it's actually asked to use; notify fails with a clear error if that one is missing. Notifier-specific credentials (Slack's tokenEnvVar) live alongside it. There's no CLI flag to override a target or a credential.

BREAKING!:
Callers need apify-test-tools.config.json to include a targets map for each notifier they use, and to invoke the CLI with its current flags: create-test-report --input <file> [--output <file>], release --output <file>, notify [--input <file>] --notifier <name> [--view dev|public]. This includes updating the github-actions-source workflows, which still use the old flag names.

[metalwarrior665]

Thanks, could you add a short description how the typical actions would change?

[metalwarrior665]

Thanks, looks good but let's have broader discussion about how the configs should work.

[metalwarrior665]

I would remove this extra notification and would just always call it "report". If someone wants to do it like we do (maybe we don't anymore) which will be super rare, they would just call notify twice. We could add something like --reportDetail detailed

[ruocco-l]

This is to create the two separate documents, these two are intended to be use with two separate notify commands, so that the elasticity comes from deciding to notify it or not, but I guess it's correct to make both of them optional, if one wants only the logging on GH.

[metalwarrior665]

I'm arguing about removing release-notify-file completely. Currently, we sort of arbitrarily post one message to #delivary-public-actors (release-slack-channel) and one to #nofit-${repo}(report-slack-channel).

Instead you could have just one notify file and the notify command would decide how to format it to Slack

[ruocco-l]

Ok, I agree that we should do one file in one file out, it's a question of how the notifier should work with this.

My idea would be that each Notifier must be able to handle a strict union of "type of notification" (as of now: failed-tests, release-public and release-detail) and those can be passed in a single file that will have to be validated and parsed by an helper function before passing it to the notifier which is still chosen by the command flag, not by what is in the config file (however we decide on its structure)

[metalwarrior665]

I haven't really read the internals yet; I'm focusing just on the commands. I think you could just pass any output file (or potentially pipe as we discussed) generated by other commands to the notify command and it would just internally format it based on the target platform and send the notification. If you pass in a different file format, we would just error out.

which is still chosen by the command flag, not by what is in the config file (however we decide on its structure)

As discussed I would try to have 1:1 equivalence in JSON config and CLI flags but it might look too ugly, let's see

[metalwarrior665]

How about unifying this as input-file and output-file for all commands? Then it is more clear which one you need. We could also by default allow piping in and out but some of the outputs are probably too big for that, let;s check

[ruocco-l]

The unification is a cool idea, we should really try and make it!
As per the piping, I actually did the files because I prefer them, I think they are easier to manage passing them from action to action.

[metalwarrior665]

I would allow at least the input piping since that doesn't dump anything into the console. Basically, the same format as the file. we can think about the std output later

Let's start by mapping each command input (not options) and output

We don't have to do it in this PR, that can be refactored later.

[metalwarrior665]

Maybe better name

Suggested change

	npx apify-test-tools report-tests \
	npx apify-test-tools create-test-report \

[metalwarrior665]

Have you thought about per Actor channels?

[ruocco-l]

I didn't, but it makes a lot of sense. I think this is something that belongs to the config file!

[metalwarrior665]

Let's just discuss this in the thread below.

[metalwarrior665]

Let's think this config file deeply through before we start adding stuff.

In high level, ideal structure for me would be that

1. All options are available per Actor - but we have to figure out what that means. E.g. for notification, you could have different channels. Then we report only changes related to each Actor to each channel? And if they share a channel, we somehow merge that? I think it is doable we have to think it through.
2. The same options are available with actor globs. e.g. groupConfigs: [{ glob: "clockworks/*", config: { tokenEnvVar: "APIFY_TOKEN_CLOCKWORKS"}]. Direct Actor-level config would have preference (or crash on conflict? or have globs only for configs?). Configs with later (or earlier?) index in the globalConfigs (naming TBD) array would have preference. I think this will be useful for e-commerce where they will have hundreds of Actors.
3. And ideally, all the same things are overridable via CLI again with glob configs. I think a neat solution would be to have the same syntax because otherwise it will be very hard to document and understand. E.g. something like --group-configs "clockworks/*.notifiers.slack.tokenEnvVar"=CLOCKWORKS_SLACK_TOKEN (probably not a valid shell)

[metalwarrior665]

Thinking about it more, I'm leaning towards having 1 and 2 non overlapping, keeping in actors only actorFullName and folder. tokenEnvVar and overrideActorContext can be in configs with globs. I cannot think now of anything that should be elsewhere than globs.

[metalwarrior665]

It might be useful to allow glob on folder, e.g. folderGlob and nameGlob since name is public and you do it for users but folder can be arbitrarily structured E.g. e-commerce has 2 types of Actors with different configs so it would be nice to glob them easily separately.

[ruocco-l]

What if we allow for groupings? A structure like this

{
  "actors": [
    {
      "tokenEnvVar": "APIFY_TOKEN_TEAM_A",
      "notifiers": { "slack": { "target": "#team-a-ci" } },
      "actors": [
        { "actorFullName": "team-a/actor-one", "folder": "actors/actor-one" },
        { "actorFullName": "team-a/actor-two", "folder": "actors/actor-two" }
      ]
    },
    {
      "tokenEnvVar": "APIFY_TOKEN_TEAM_B",
      "notifiers": { "slack": { "target": "#some-channel" } },
      "actorFullName": "org/standalone-actor",
      "folder": "actors/standalone-actor"
    }
  ]
}

Will allow us to understand "If actors are in the same grouping they go together when it comes to notification/reporting" unless there is a specific override of the properties in the top level (e.g. a specific channel for the reporting of one specific actor)

[metalwarrior665]

That's also possible, a few cons are

• It would be quite annoying if you have group A use token A, then group B use token B. And then half of group A uses notifier A and other half notifier B. We would then need to illogically chunk it into smaller groups. So in this regard the separate glob based configs are more flexible.
• Would you still want to have the glob override on the CLI command? Since overriding this via command will be quite involving. And if we do globs in the apify-test-tools.config.json we will use the same parser for it.

[ruocco-l]

It would be quite annoying if you have group A use token A, then group B use token B. And then half of group A uses notifier A and other half notifier B. We would then need to illogically chunk it into smaller groups. So in this regard the separate glob based configs are more flexible.

I don't think the chunking is needed. With 6 actors it would be like

{
  "actors": [
    {
      "tokenEnvVar": "TOKEN_A",
      "notifiers": { "slack": { "target": "#team-a-default" } },
      "actors": [
        { "actorFullName": "team-a/actor-1", "notifiers": { "slack": { "target": "#team-a-special" } } },
        { "actorFullName": "team-a/actor-2" },
        { "actorFullName": "team-a/actor-3" }
      ]
    },
    {
      "tokenEnvVar": "TOKEN_B",
      "notifiers": { "slack": { "target": "#team-b-default" } },
      "actors": [
        { "actorFullName": "team-b/actor-4" },
        { "actorFullName": "team-b/actor-5", "notifiers": { "slack": { "target": "#team-b-special" } } },
        { "actorFullName": "team-b/actor-6" }
      ]
    }
  ]
}

[metalwarrior665]

even if it might make shit easier for e-commerce.

It is not just e-commerce; we want to offer this for the community, and they also might have large monorepos.

Maybe some root notifier and per-actor override? (yeah it might be too much with 100+ actors tho).

I think the per Actor overrides also can get quite hidden in the large list.

With globbing you have to check each pattern, also what would happen if 2+ patterns match an entry?

Probably later glob trumps earlier, not very clean ofc. But I don't expect some super complex combos, it would be maybe 5 configs per repo.

Extra: maybe a per-actor disable of notify is nice for e-commerce, since they dont really care that much about clanker-made actors.

We should add --actors and --ignore generically to all commands

[metalwarrior665]

I don't think the chunking is needed. With 6 actors it would be like

That looks pretty good. How (if) would you override it through CLI? E.g. with the release double notification

[ruocco-l]

If we keep the idea from this, channel release channels would also belong in the config and for the CLI I wouldn't do any support other than a flag (that can stay --target) overriding all the channels to the same one, just for local development

[JuanGalilea]

can we meet about this?
lets go nuts on the notion pitching shit

I have this feeling we could benefit of thinking about this in a deeper level, not just tweaking the config for this new feature, and then again for the next one, etc.

[Patai5]

I'm not sure how much this is needed. IMO this is not a big deal at least for us, and I doubt that it will be fore some community developer either.

If they need some release notes, then they can probably do so better in their own CI 🤷

---

Otherwise for you to consider: personally for a top level design of the code, it would be really nice to have a class interface for the Notifier class rather than a raw function. It would be nice for extendability for community developers. And for the assrtions for the configurations (e.g. the slack token assertion) could be done directly in the class constructor, rather than only on runtime of the function.

Thanks!

[metalwarrior665]

How about we pass the vitest report to notify directly and parse it there? What would be the advantage of this middleman command?