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

Make change ids in tests repeatable #964

Merged
merged 8 commits into from
Jan 4, 2023
Merged

Make change ids in tests repeatable #964

merged 8 commits into from
Jan 4, 2023

Conversation

ilyagr
Copy link
Collaborator

@ilyagr ilyagr commented Jan 1, 2023

This will be needed to test functionality for showing shortest
unique prefix for commit and change ids. As a bonus, this also
allows us to test log output with change ids.

As another bonus, this will prevent occasional CI failures like
https://github.com/martinvonz/jj/actions/runs/3817554687/jobs/6493881468.

Checklist

If applicable:

  • I have added tests to cover my changes

@ilyagr ilyagr marked this pull request as ready for review January 2, 2023 00:02
@ilyagr ilyagr force-pushed the ig/repeatable-ids branch 6 times, most recently from c97e704 to 6b45d6f Compare January 2, 2023 06:02
lib/src/settings.rs Outdated Show resolved Hide resolved
lib/src/revset.rs Outdated Show resolved Hide resolved
lib/src/commit_builder.rs Outdated Show resolved Hide resolved
lib/src/commit_builder.rs Outdated Show resolved Hide resolved
lib/src/commit_builder.rs Outdated Show resolved Hide resolved
lib/src/commit_builder.rs Outdated Show resolved Hide resolved
@martinvonz martinvonz mentioned this pull request Jan 2, 2023
3 tasks
The timestamp couldn't be set with `--config-toml 'user.timestamp=...'`.
It makes more sense for this function to change `self`.
@ilyagr ilyagr force-pushed the ig/repeatable-ids branch 5 times, most recently from 1616859 to 2571f82 Compare January 2, 2023 21:48
lib/src/settings.rs Show resolved Hide resolved
lib/src/settings.rs Show resolved Hide resolved
lib/src/commit_builder.rs Outdated Show resolved Hide resolved
Copy link
Owner

@martinvonz martinvonz left a comment

Choose a reason for hiding this comment

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

This looks good as far as I'm concerned, so just see how @yuja feels about the remaining unresolved comments.

lib/src/index.rs Outdated Show resolved Hide resolved
lib/src/commit_builder.rs Outdated Show resolved Hide resolved
@ilyagr
Copy link
Collaborator Author

ilyagr commented Jan 4, 2023

@yuja, are you OK with the current state of this? There's no rush, I'm just checking if you're waiting on me or I'm waiting on you.

@yuja
Copy link
Collaborator

yuja commented Jan 4, 2023

@yuja, are you OK with the current state of this?

Can you remove scary comment about if new_rng_seed != get_rng_seed_config(&self.config)?
I'll need to rework that to load repo config, and this bits will be a blocker.

Other than that, this PR looks good.

Some hijinx are utilized to make it possible to generate
random values using an immutable `&UserSettings` reference.
Moves all the config that depends on the command
number next to each other.
In the following commit, we replace it everywhere else.
This will be needed to test functionality for showing shortest
unique prefix for commit and change ids. As a bonus, this also
allows us to test log output with change ids.

As another bonus, this will prevent occasional CI failures like
https://github.com/martinvonz/jj/actions/runs/3817554687/jobs/6493881468.
@ilyagr ilyagr enabled auto-merge (rebase) January 4, 2023 06:51
@ilyagr ilyagr merged commit b293d72 into main Jan 4, 2023
@ilyagr ilyagr deleted the ig/repeatable-ids branch January 4, 2023 07:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants