Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Performance Tests Workflow: Polish Bash script #32284

Merged
merged 3 commits into from
May 27, 2021

Conversation

mcsf
Copy link
Contributor

@mcsf mcsf commented May 27, 2021

Description

This PR consists of three commits:

Avoid non-POSIX grep -P

grep -P is a GNU extension to use PCRE regular expressions. While it is commonly available on GNU/Linux systems, it is not POSIX and cannot be guaranteed to exist. By replacing it with POSIX-compliant AWK, this workflow is easier to test locally on different systems, including all BSD-derived ones.

Use safer or more modern Bash

  • Prefer $(( ... )) over antiquated expr
  • Use double quotes to prevent globbing and word splitting
  • Condense read options... just because

As a general note, Shellcheck is an excellent tool for linting and validating shell scripts.

Explicitly require Bash shell

The run block for the "Compare performance with current WordPress Core and previous Gutenberg versions" job relies on a series of Bashisms (e.g. <<< syntax, arrays), so we should make that an explicit requirement.

Types of changes

Checklist:

  • My code is tested.
  • My code follows the WordPress code style.
  • My code follows the accessibility standards.
  • I've tested my changes with keyboard and screen readers.
  • My code has proper inline documentation.
  • I've included developer documentation if appropriate.
  • I've updated all React Native files affected by any refactorings/renamings in this PR (please manually search all *.native.js files for terms that need renaming or removal).

mcsf added 3 commits May 27, 2021 17:06
`grep -P` is a GNU extension to use PCRE regular expressions. While it
is commonly available on GNU/Linux systems, it is not POSIX and cannot
be guaranteed to exist. By replacing it with POSIX-compliant AWK, this
workflow is easier to test locally on different systems, including all
BSD-derived ones.
* Prefer `$(( ... ))` over antiquated `expr`
* Use double quotes to prevent globbing and word splitting
* Condense `read` options... just because

As a general note, Shellcheck [1] is an excellent tool for linting and
validating shell scripts.

[1]: https://www.shellcheck.net/
The `run` block for the "Compare performance with current WordPress Core
and previous Gutenberg versions" job relies on a series of Bashisms, so
we should make that an explicit requirement.
@mcsf mcsf added [Type] Automated Testing Testing infrastructure changes impacting the execution of end-to-end (E2E) and/or unit tests. [Type] Code Quality Issues or PRs that relate to code quality labels May 27, 2021
@mcsf mcsf requested a review from ockham May 27, 2021 16:24
@ockham ockham merged commit 938fc9e into trunk May 27, 2021
@ockham ockham deleted the fix/workflow-performance-bash branch May 27, 2021 18:15
@github-actions github-actions bot added this to the Gutenberg 10.8 milestone May 27, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Type] Automated Testing Testing infrastructure changes impacting the execution of end-to-end (E2E) and/or unit tests. [Type] Code Quality Issues or PRs that relate to code quality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants