Skip to content

Conversation

MaxDesiatov
Copy link
Contributor

Use of --clean can lead to irreversible loss of uncommitted data. It is still desirable to reset Swift project repositories to a clean state, but without deleting all of the in-progress changes. Passing --stash instead of (or in addition to) --clean will preserve uncommitted changes in stashes of corresponding repositories instead of completely deleting those.

Use of `--clean` can lead to irreversible loss of uncommitted data. It is still desirable to reset Swift project repositories to a clean state, but without deleting all of the in-progress changes. Passing `--stash` instead of (or in addition to) `--clean` will preserve uncommitted changes in stashes of corresponding repositories instead of completely deleting those.
@MaxDesiatov MaxDesiatov added update-checkout Area → utils: the `update-checkout` script contributor experience labels Jan 26, 2024
@MaxDesiatov
Copy link
Contributor Author

@swift-ci smoke test

@MaxDesiatov
Copy link
Contributor Author

@swift-ci smoke test macos

@MaxDesiatov MaxDesiatov enabled auto-merge (squash) January 27, 2024 16:43
@AnthonyLatsis AnthonyLatsis added the utils Area: the build system and other accessory scripts under the "utils" directory label Feb 13, 2024
Copy link
Collaborator

@AnthonyLatsis AnthonyLatsis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. A test would be nice.

@MaxDesiatov MaxDesiatov added the needs tests Flag: Issue is fixed but needs tests / PR needs tests label Feb 13, 2024
@MaxDesiatov
Copy link
Contributor Author

@swift-ci smoke test

@MaxDesiatov
Copy link
Contributor Author

@swift-ci smoke test

@MaxDesiatov MaxDesiatov merged commit 22a37e3 into main Mar 21, 2024
@MaxDesiatov MaxDesiatov deleted the maxd/update-checkout-stash branch March 21, 2024 19:17
@MaxDesiatov
Copy link
Contributor Author

LGTM. A test would be nice.

Can you provide some details on what would be the best way to cover this with tests, in your opinion?

@AnthonyLatsis
Copy link
Collaborator

AnthonyLatsis commented Mar 22, 2024

update-checkout has a Python unit test suite at utils/update_checkout/tests. You can run it using utils/update_checkout/run_tests.py or lit + validation-test/Python/update_checkout.swift. The test suite is centered around an intermediary unittest.TestCase subclass from which all the effective test classes inherit. This base class is responsible for setting up and tearing down a mock-up environment consisting of a directory of local remotes and a source root — a directory for their clones. Subclasses define test functions that run update-checkout commands against these mock repositories and perform state checks.

I think a single test function that

  1. clones the remotes using update-checkout
  2. creates some untracked files in several clones
  3. runs update-checkout with --stash
  4. verifies that the untracked files have indeed been stashed and nothing else happened

would be good enough for now (the update/clone tests, if any, are really basic anyway). I would place this function in a new subclass in its own file.

@AnthonyLatsis AnthonyLatsis removed the needs tests Flag: Issue is fixed but needs tests / PR needs tests label Mar 22, 2024
harshitapath pushed a commit that referenced this pull request Mar 27, 2024
Use of `--clean` can lead to irreversible loss of uncommitted data. We still need to reset Swift project repositories to a clean state, but without deleting all in-progress changes. Passing `--stash` instead of (or in addition to) `--clean` will preserve uncommitted changes in stashes of corresponding repositories.
shahmishal added a commit that referenced this pull request Mar 28, 2024
Add `--stash` as alternative to `--clean` to `update-checkout` (#71178)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
contributor experience update-checkout Area → utils: the `update-checkout` script utils Area: the build system and other accessory scripts under the "utils" directory
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants