[SPARK-58573][INFRA][BUILD] Replace defunct Jenkins test grouping with modern GitHub Actions grouping

[nchammas]

What changes were proposed in this pull request?

Remove the old CURRENT_BLOCK and ERROR_CODES testing infrastructure and replace it with an updated titled_block context manager that also groups log output on GitHub Actions. Move section titles from the various helper functions into titled blocks in main(). Exit codes are no longer driven by the old blocks. If the tests fail, the script exits with a non-zero code, usually 1.

Make sure that titled blocks cannot be nested, and that all large blocks of output have a suitable title block at the top level main() function. That means I moved the following checks and calls up to main():

• run_java_style_checks
• build_spark_unidoc_sbt
• SKIP_PACKAGING flag check
• SKIP_UNIDOC flag check

Fold group_in_github_actions into the new titled_block.

Remove build_spark_documentation, which is not used by anything.

Why are the changes needed?

Amusingly, I added this testing infrastructure 12 years ago in #2606 (later refactored into Python in #5694). It was built for Jenkins, which we do not use anymore. The block and error codes are not used by any part of our current test infrastructure on GitHub Actions.

The new titled_block context manager takes care of printing block titles as before, and also subsumes the log grouping behavior from group_in_github_actions (introduced in #54524) since it is useful for more than just sbt.

We have a separate job for building the docs, so I assume we don't want to revive the currently-dead build_spark_documentation.

Does this PR introduce any user-facing change?

No.

How was this patch tested?

CI for this PR. Some illustrative screenshots of the new log groups:

The purple debug lines are from my fork and are not related to this PR.

Was this patch authored or co-authored using generative AI tooling?

I wrote this with assistance from GitHub Copilot.

[nchammas]

cc @gaogaotiantian since you authored #54524.

[uros-b]

Thank you @nchammas! Adding @gaogaotiantian for further review

[gaogaotiantian]

Overall I think this is a good direction. For the code itself I only have a minor comment. Could you include all the changes you made in the description though? It took me a while to figure out why some of the stuff is moved around. The style thing, the packaging test. I think moving them makes sense but it would be nice to mention these in the description (commit message when the PR is merged) so it's easier to track in the future.

[gaogaotiantian]

Let's bind this guard to the function itself.

if getattr(titled_block, "_entered", False):
    raise XX
titled_block._entered = True
try:
   yield
finally:
   titled_block._entered = False

[nchammas]

Hmm, that's a neat technique. I see asyncio uses it, though for class instances. If you insist, I'll adopt your suggestion.

But isn't a global variable more suitable here? It's a much more common technique in user code, and in this case we are controlling a "top-level" invariant of the script.

[gaogaotiantian]

The point it to avoid polluting global name space. If a variable is only used by a single function, it should be part of that function. I'm okay if you want to make this function a class so you can properly define _entered as a class attribute, but I don't like having global variables for a single function. It's not obvious that _in_titled_block is designed for titled_block only. It's possible that in the future someone accidentally insert some code between _in_titled_block and titled_block which will make it more difficult to catch the fact. It's also possible that someone uses _in_titled_block in an unexpected way because it's a global variable and conflict the existing usage.

I don't think this is a huge deal, but I think it's a safer way to implement this guard. I know global variables are a more common way to do things like this, but most users write their code for convenience, not stability.

[nchammas]

Could you include all the changes you made in the description though? It took me a while to figure out why some of the stuff is moved around. The style thing, the packaging test.

This is what I was referring to with this line from the PR description:

Make sure that title blocks cannot be nested, and that all large blocks of output have a suitable title block at the top level main() function.

But I will update it to be more explicit about what got pulled up to main(). My bad.