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

[Feature]: Support "real" TCR that also reverts tests #674

Closed
82 tasks done
philou opened this issue Jun 3, 2024 · 1 comment · Fixed by #792
Closed
82 tasks done

[Feature]: Support "real" TCR that also reverts tests #674

philou opened this issue Jun 3, 2024 · 1 comment · Fixed by #792
Assignees
Labels
enhancement New feature or request M Medium workflow

Comments

@philou
Copy link
Contributor

philou commented Jun 3, 2024

Contact Details

[email protected]

Feature Request

The TCR tool only reverts the code files, and keeps the failing test. The original TCR was also reverting the tests. These 2 behaviors lead to quite different workflow. The original TCR encourages:

  • to add "disabled" tests that get "re-disabled" when they fail
  • to take even smaller steps, and to use the tidy-first workflow: "Make the change easy, then make the easy change"

Feature Description

This could be an option to TCR (ex: --also-revert-tests). It could also become the default behavior, and make the current behavior available with an option (ex: --nice)

Alternatives

An alternative is to completely drop the current behavior, and stick only to the Real TCR.

Additional Context

I needed this feature on the Baby Steps Golf Kata (https://github.com/philou/kata-babystepsgolf). I ended up reverting to straightforward TCR scripts. I also needed to run the TCR manually (and not systematically on save, as I wanted participants to give a special commit message).

Code of Conduct

  • I agree to follow this project's Code of Conduct

TODO

  • Add a parameter "flavor": taking 2 values: "nice" (current, and default) and "original" (which reverts tests)
    • find a shortcut letter for the command line -> 'f'
    • make commit failures shortcut upper 'F'
    • define the flavor parameter
    • Call the AddFlavorParam
  • add and pass down the flavor attribute to the TCR struct
    • add flavor to TCREngine
    • pass the flavor from UI to TCREngine
  • Display the flavor in the UI
    • Terminal
    • Angular
      • Transfer Flavor through HTTP
      • Display in in the web frontend
      • Improve wording on web frontend
  • Transform flavor from string to enum:
    • flavor -> Variant
    • nice -> relaxed
    • original -> btcr (build && (test && commit || revert)
    • Create an enum
    • propagate the types from the params
    • add a Name() function to variant type
  • in tcr.revert, branch on the 2 variants
    • Add a test around the current source revert, asserting reverted files with VCS fake
      • test that revert was called
      • add both tests and source files
      • make it possible to capture src from test files in the languages
      • assert that only source files are reverted
      • override and add a parameter to initTCREngineWithFakes to pass in an array of file diffs
      • assert that many sources files can be reverted
      • Transform the tests to parametrized tests
    • Rename tcr.srcRevert to tcr.revertSrcOnly
    • implement the btcr variant
      • add tests around btcr
      • implement
  • add a 3rd variant: "introspective", which is based on btcr but also commits failures and reverts
    • draw a mermaid diagram to visualize what it will look like
    • add description for introspector variant in variants markdown description file
    • check that it works in the UI and Command Line
    • Add tests for the introspective variant
      • Add a list of received commands in VCS fake so that we can test that we got "commit && revert"
      • Should we update the LastCommand to get the tail of the list instead of keeping it?
      • Unskip the test with "add, commit, rollback, and commit"
    • in tcr.revert, implement the introspective variant
      • implement introspectiveRevert (copied from git commitFailures)
      • make sure we have tests for the commit messages
      • clean up the revert function (currently spanning different levels of abstraction)
    • And what about the error we are ignoring out of introspectiveRevert?
    • Add introspective to the list of variants in the short description for variant parameter
  • Reinforce command line argument "-r" so that in only accepts recognized variant names
  • remove the "commit failure" option and code
    • remove config/cparam_commit_failure()
    • remove CommitFailures flag from params and related test data builder
    • remove parts that leverage on CommitFailures flag in engine.tcr
    • remove SetCommitOnFail()
      • in trc class
      • in tcr interface
    • remove from checker.check_workflow
  • clean up vcs interface and related git/p4 implementations
  • add a link to the workflow picture from the web app session info
  • Rename in VCS to map to TCR domain, not that of git:
    • rename Revert into RollbackCommit
    • rename Restore into Revert
Variants TCR VCS (current) VCS (updated) git p4
original, BTCR, relaxed revert(file) Restore(file) RevertLocal(file) git checkout HEAD file p4 reconcile file
introspective rollbackLastCommit()? Revert() RollbackLastCommit() git revert HEAD TODO: p4 undo + p4 submit
  • Test the introspective revert in practice.
    • Should we add some warning or info traces in introspective mode? No, confirmed that this cannot happen when using introspective variant
    • Tune git revert command call so that it does not auto-commits
  • rework checker for TCR workflow configuration
    • rename chech_workflow group to check_variant
    • add checking of variant value
      • variants btcr, relaxed and introspective should be supported (lowercase or uppercase)
      • variant original should return a "not yet supported" error message
      • all other cases should return an error
    • remove "commit failure" related stuff from checker
  • document in readme each of the available variants and what they imply in terms of workflow (explain ref to https://medium.com/@tdeniffel/tcr-variants-test-commit-revert-bf6bd84b17d3)
    • Create visual workflows for each variant
  • Remove the "amend" parameter in the VCS.Commit function

Parking

@philou philou added enhancement New feature or request M Medium workflow labels Jun 3, 2024
@mengdaming
Copy link
Contributor

A note about running TCR manually (and not systematically on save): this part is already available through running tcr one-shot subcommand

philou added a commit that referenced this issue Aug 13, 2024
- add a test for SessionInfo
- add a TCR.SetFlavor(...) method to interface
philou added a commit that referenced this issue Aug 13, 2024
- pass Flavor param to TCREngine
- print the Flavor in Terminal.ShowSessionInfo
- add "nice" flavor by default in fakes and test doubles
philou added a commit that referenced this issue Aug 13, 2024
- pass the Flavor through HTTP
- update the html to display the flavor
- adapt structs and tests
aatwi added a commit that referenced this issue Aug 13, 2024
- Big grep replace
- Update docs
aatwi added a commit that referenced this issue Aug 14, 2024
- Rename nice to relaxed
- Rename original to btcr
aatwi added a commit that referenced this issue Aug 14, 2024
- Implement a new Variant enum
- Replace the usage of relaxed and btcr by the enum values
aatwi added a commit that referenced this issue Aug 14, 2024
- Implement the function Name that returns the value of the Variant as a string
- Use the function in the code
aatwi added a commit that referenced this issue Sep 11, 2024
- Check whether a file contains a 'test' to know if it is a test file
- Added tests for different revert configuration of the relaxed TCR
aatwi added a commit that referenced this issue Sep 11, 2024
- Replace tests by parameterized tests
- Added new test that covers not reverting test files
mengdaming added a commit that referenced this issue Sep 11, 2024
- Add tests
- Inline revertSrc() file into revert()
- Extract shouldRevertFile()
- Extract noFilesRevertedMessage()
mengdaming added a commit that referenced this issue Sep 13, 2024
mengdaming added a commit that referenced this issue Oct 1, 2024
@github-project-automation github-project-automation bot moved this from In progress to Done in TCR Enhancements & Bugs Follow Up Oct 1, 2024
mengdaming added a commit that referenced this issue Oct 1, 2024
- create variant-introspective.mmd
- update other diagrams to add "VCS" on VCS-related actions
- generate png files
mengdaming pushed a commit that referenced this issue Oct 1, 2024
To document what this new variant does
- add a section
- add a description of the variant
- also add an extract from T Deniffel's post to explain what the
  other variants are
mengdaming pushed a commit that referenced this issue Oct 1, 2024
So that users can input variant=introspective
- add introspective to variant enum
- make sure web app understand the introspective variant
mengdaming added a commit that referenced this issue Oct 1, 2024
- add test placeholder for param variant value checking
- add test placeholder for introspective variant execution by engine
mengdaming added a commit that referenced this issue Oct 1, 2024
- change param.variant type to string to be consistant with other params handling
- remove config/param_variant_test.go
- add variant.Select(string) to manage variant instance selection
- add UnsupportedVariantError (returned when variant name is not recognized)
- ensure TCR engine exits on error when variant is not recognized
mengdaming added a commit that referenced this issue Oct 1, 2024
mengdaming added a commit that referenced this issue Oct 1, 2024
- add a list of received commands in VCS fake so that we can test that we got "commit && revert"
- add vcsfake.GetLastCommands(n) to get the tail of the list of last commands
- add vcsfake.VerifyLastCommandsAre() as an alternative approach
mengdaming added a commit that referenced this issue Oct 1, 2024
- remove vcsfake.VerifyLastCommandsAre() alternative approach
mengdaming pushed a commit that referenced this issue Oct 1, 2024
To have clearer names for all VCS
- rename interface function
- adapt fake
mengdaming pushed a commit that referenced this issue Oct 1, 2024
To have clearer names for all VCS
- rename interface function
- adapt fake
mengdaming pushed a commit that referenced this issue Oct 1, 2024
To clean up code now that we don't support commit failures
mengdaming added a commit that referenced this issue Oct 1, 2024
- add tcr.introspectiveRevert()
- inject call to introspectiveRevert() in tcr.revert()
- tune and pass test case on revert for introspective variant
mengdaming pushed a commit that referenced this issue Oct 1, 2024
- Clean the tcr.revert function
- Added an array in vcs fake to track the commit subjects
- Add tests to verify the output of messages
mengdaming pushed a commit that referenced this issue Oct 1, 2024
Because we don't need it anymore without the 'commit failure' option
- remove parameter from interface
- adapt implementations
- simplify tests
- adapt calls
mengdaming pushed a commit that referenced this issue Oct 1, 2024
- refactor simple and introspective Reverts to simplify error handling
- add a test to make sure we deal with VCS errors in introspectiveRevert
mengdaming added a commit that referenced this issue Oct 1, 2024
So that we can use our own commit message when reverting
- add option "--no-commit" to git revert command
- tune git package tests accordingly
mengdaming pushed a commit that referenced this issue Oct 1, 2024
- Implement the function variant.checkVariant
- Rename checkWorkflowConfiguration to checkVariantConfiguration
- Add tests to verify the function variant.checkVariant
- Allowed variant names to be case-insensitive
mengdaming added a commit that referenced this issue Oct 1, 2024
- renamed test cases from restore to revert_local
- renamed git test case from revert to rollback_last_commit
- added p4 rollback_last_commit test case (wip)
mengdaming added a commit that referenced this issue Oct 1, 2024
- removes --commit-failures flag
- updates --variant flag description
philou added a commit that referenced this issue Oct 8, 2024
To experiment tracking technical debt in the code.
- add FIXME comments for 2 smells we stumbled on when coding #674
mengdaming pushed a commit that referenced this issue Oct 11, 2024
To experiment tracking technical debt in the code.
- add FIXME comments for 2 smells we stumbled on when coding #674
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request M Medium workflow
Development

Successfully merging a pull request may close this issue.

3 participants