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

Potential GitHub automation workflow/script improvements #384

Open
jscholes opened this issue Feb 2, 2021 · 3 comments
Open

Potential GitHub automation workflow/script improvements #384

jscholes opened this issue Feb 2, 2021 · 3 comments
Labels
Agenda+Community Group To discuss in the next workstream summary meeting (usually the last teleconference of the month) enhancement New feature or request tests About assistive technology tests

Comments

@jscholes
Copy link
Contributor

jscholes commented Feb 2, 2021

The GitHub Action, triggered by pushing to a PR branch, currently executes the create-all-tests and review-tests scripts. These cause all assets to be regenerated for all patterns present in the tests directory, even though a test developer will typically only be working on one pattern at a time.

As the number of patterns in the tests directory increases, so will the running time of the scripts and workflow. The current approach also increases the likelihood of merge conflicts (e.g. like those seen in PR #330), because files are updated which have no relation to the test pattern under development.

Additional Observations

  1. The GitHub workflow runs indiscriminately, regardless of context. For example:
    • If a developer manually runs the create-tests script locally, to only regenerate assets for a specific pattern, the create-all-tests script will run anyway on push.
    • If a pull request is submitted which has nothing to do with test writing (e.g. an editorial update to the README or other unrelated file(s)), all tests will be regenerated and a preview link added to the PR.
  2. All pattern assets are regenerated in full, irrespective of which changes have been made since the last generation. E.g. files for all 76 "Editable Combobox With Both List and Inline Autocomplete " tests are recreated, even if a small change is made to only one of the rows in tests.csv.

Suggested Enhancements

In priority order:

  1. Automatically detect, or allow a test developer to manually specify, the applicable patterns on a particular branch, and only regenerate assets for those patterns.
    • This ties in with issue Pull request preview link enhancements #371 (enhancements for PR preview links).
    • If a PR requires all tests to be regenerated for some reason, developers should have a way of specifying such.
    • If a PR contributor doesn't specify some or all patterns for regeneration, the GitHub workflow shouldn't run.
  2. Detect when a developer has already generated test assets locally, to avoid unnecessary execution of the GitHub workflow and the git push followed by git pull dance.
  3. Implement a differential strategy to avoid regenerating all test assets when only small, targeted changes have been made.
@jscholes jscholes added enhancement New feature or request Agenda+Community Group To discuss in the next workstream summary meeting (usually the last teleconference of the month) tests About assistive technology tests labels Feb 2, 2021
@s3ththompson
Copy link
Member

s3ththompson commented May 26, 2021

Here are the proposed steps we discussed in the last CG meeting:

  1. Put verbose logging behind flag
  2. Summarize errors with better line & file explanations
  3. Add ability to validate only
  4. Add ability to build/validate single test plan given as argument
  5. Turn on Netlify continuous deployment and PR deploy previews
  6. Add conditional logic in app to use new Netlify previews if commit is newer than the the present (and continue loading from GitHub repo for backwards compatibility)
  7. Remove generated files from repository
  8. Speed up test generation by only building one template that renders with json

@jscholes
Copy link
Contributor Author

@s3ththompson Thanks for this summary. Given that items 5 through 8 would resolve the blocking concerns with PR merges, would it be possible to prioritise those over 1 through 4? Presumably, the build commands can remain the same even when 1 through 4 have been implemented, so they aren't a blocker to getting Netlify previews up and running.

@s3ththompson
Copy link
Member

@jscholes, that makes sense.

i just had a conversation with @howard-e and @jesdaigle. I think we may modify the above steps even more to prioritize a short-term fix for the merge issue. I'll report back shortly to keep this thread up to date. @howard-e is picking up this work given that @mzgoddard is on vacation this week--we want to solve this issue before he returns.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Agenda+Community Group To discuss in the next workstream summary meeting (usually the last teleconference of the month) enhancement New feature or request tests About assistive technology tests
Projects
None yet
Development

No branches or pull requests

2 participants