This adds another property-based test which tests for a different
property: after applying only some of the changes in a diff, does the
application of the remaining changes result in the "right" tree?
There are some caveats which changes can be safely selected without
breaking this "roundtrip" property, explained in the doc comment.
Inferring the complementary change selection in the format expected by
scm_report is also more tricky than one might naively expect.
The selection is also subject to random generation and is represented by
a `Vec<bool>` that is wrapped around the actual state machine and that
is successively drawn from to decide which change to select. Otherwise,
the presence of this selection mask does not affect the state machine.
I chose `Vec<bool>` other an actual numeric bitmask because proptest
shrinks the vector more sensibly.
This adds the proptest crate for property-based testing as well as the
proptest-state-machine crate as direct dev dependencies of jj-cli and as
dependencies of the internal testutils crate.
Within testutils, a `proptest` module provides a reference state
machine which models the working copy as a map from path to `DirEntry`.
Directories are not represented explicitly, but are implicit in the
ancestors of entries.
The possible transitions of this state machine are for now limited to
the creation of new files (including replacements of existing files
or directories) and a `Commit` operation which the SUT can use to
snapshot a reference state. Additional transitions (moving files,
modifying file contents incrementally, ...) and states (symlinks,
submodules, conflicts, ...) may be added in the future.
This reference state machine is then applied to the builtin merge-tool's
test suite:
- The initial state is always an empty root directory.
- The `Commit` operation creates `MergedTree` from the current state.
- Each step of the way, the same test logic as in the manual
`test_edit_diff_builtin*` tests is run to check that splitting off
none or all of the changes results in the left or right tree,
respectively. The "right" tree corresponds to the current state,
whereas the "left" tree refers to the last "committed" tree.
Co-authored-by: Waleed Khan <me@waleedkhan.name>
This macro in the style of `assert_eq!` compares two trees based
on their `MergedTreeId`s. In case they do not compare equal, the
corresponding trees are dumped in the panic message.
Like `assert_eq!`, the macro accepts a custom format string which will
be included in the panic message.
This was discussed in #3219 and #3262, and apparently, people don't agree
whether we should implement machine-readable formatting by using templater.
However, there are recurring requests about JSON output, and "jj api" is still
vaporware. So I think it's good to add some support for JSON output.
I'm not going to add syntax to build custom JSON object/list for now. Instead,
property types such as Commit will implement fixed serialization output. The
user might have to write ad-hoc JSON constructs to concatenate the default JSON
output and their custom JSON fields.
We'll need a documentation about which types are serializable. I think this can
be added later when more types get serialization support.
The conversion is implemented for basic types for now. More types will become
serializable.
Instead of .try_into_serialize(), we can add .try_into_json() method, which
wouldn't require erased-serde. However, I think the separation of serializable
types and serialization formats is nice. .try_serialize<S>(serializer: S) might
also work, but I haven't tried. It would be complicated since template property
is basically a thunk of Fn() -> Output type.
When experimenting on hidden revisions extension in revset #5871, I noticed "jj
abandon" attempted to re-abandon hidden revisions. This could be worked around
by repo.transform_descendants(), but I'm not sure whether the repo API should
ignore hidden revisions which are explicitly specified.
In any case, I don't think "jj abandon" should say hidden revisions get
abandoned.
The output looks better if the graph had long parallel history. "--limit=N" is
applied after sorting for consistency with "jj log". The doc also mentions that.
Since TopoGroupedGraphIterator emits predecessors in reverse order at squash
point, we no longer need to tweak the visiting order by walk_predecessors().
There's no technical reason to restrict the starting revisions to a single
commit, and it will be nice that we can see evolution history of duplicated,
split, or divergent commits.
We use a query `heads | (domain & ::heads & files(path))` in annotation process,
and it was silly that the whole expression was filtered within `all()`. We
should instead evaluate `heads | filter_within(domain & ::heads, files(path))`.
Fixes#6713
Since divergence doesn't mean the commit has been squashed from / split into
multiple commits, it doesn't make much sense to show divergent commits as a
single entry. So this patch turns "changes" into a map indexed by CommitId in
order to track divergent commits individually. The ModifiedChange type is also
reorganized accordingly.
If old commits had divergence, we can't reliably associate new commit with the
old one. In the original implementation, we worked around the problem by not
displaying diffs for such changes. In this patch, the latest "old" commit is
arbitrarily chosen. I think this is better than disabling diffs, and will help
integrating predecessors information. Change-id based deduction won't be used
if new commits exist in the predecessors map.
This basically means that the graph is sorted by "new" commits, then abandoned
commits follow. I'm thinking of integrating predecessors information into "op
diff", and it will make sense to identify changes by commit ids instead of
change ids there. Divergent commits will be displayed individually, and squashed
commits will have all predecessor commits including ones having different change
ids.
This should also fix "graph has cycle" issue. In the original implementation, we
mixed parent change ids of both new and old views, which could result in cycle.
Unfortunately, we can't reproduce a merge in bit-exact manner. The shape of the
commits graphs should be identical, but commit/change ids differ. I don't think
we would want to see the list of "identical" changes, so this patch disables
show_op_diff() for merge operations. "op diff" is unchanged because it doesn't
fail, and the diff functionality might be useful for debugging problems.
Alternatively, we could show diff from the auto-merge state without rebasing
descendants, but I don't think that would be useful either. The state before
rebase_descendants() isn't visible to users, so it shouldn't be the stuff the
user would care about.
Fixes#4465
Follows up 708e1c58cd "cli: improve hint message when suggesting
`--ignore-immutable`." Not all callers of find_immutable_commit() need
lower/upper bounds, and this seems better in API standpoint.
This patch adds a `TreeValue::File::copy_id` field. The copy ids are
always empty for now. I preserved the copy id where it was easy to do
so, plus in a few non-trivial cases. In other places, however, I made
the code use a new copy id. I added a `CopyId::placeholder()`
function for creating a new copy id where we need one. We should
eventually fix all callers to either preserve an existing copy or to
generate a new one.
This patch implements the methods only in the test backend. That
should enough for us to start implementing diffing and merging on top,
including tests for that functionality. Support in the Git backend can
come once we've seen that the model works.
When splitting a commit which both changes a binary file and flips
the executable bit, selecting just the `Binary` change but not the
`FileMode` change still applied the mode change.
This is now fixed by overriding the file mode of a selected binary file
with whatever file mode scm-record tells us was selected.
If an empty file or a binary file has its file mode (specifically the
executable bit) changed but its contents remain the same, scm-record
reports its contents as `Unchanged`. By not doing anything for unchanged
_contents_, a potentially selected file mode change was lost.
For text file, scm_record 0.8.0 currently always reports
`SelectedContents::Text`, even if the file hasn't changed at all (or
only its executable bit has). I've created arxanas/scm-record#104
upstream to track this bug. This unexpected behavior caused file
mode-only changes to be handled correctly but it unnecessarily rewrote
the file object, where writing the tree would have sufficed. Thus,
fixing the upstream issue prior to this commit would exacerbate the
issue by also affecting text files.
This commit now addresses file mode changes by updating the `TreeValue`
without rewriting the files or generating new file IDs. It is sufficient
to do so only if a file mode change has been selected, reusing the same
`file_mode_change_selected` condition introduced by the previous commit.
When the contents of a binary file have not changed (but it appears in
the tree diff because its file mode has) the builtin diff tool still
presented a `Section::Binary` section to the user, showing identical
descriptions for both sides of the diff.
Selecting this section should not affect the selected contents, so it
does not make much sense.
This behavior was not captured by any of the existing snapshot tests but
it will be captured by a new regression test added in the next commit.
This adds a manual regression test for the scenario in issue #5189.
As of #6411, this no longer results in a panic. However, the test still
fails and rightfully so: When following the reproduction for #5189 and
not selecting any change in `jj split`, the split-off (first) commit
will still record the deletion of `folder/.keep` but not the creation of
the file `folder`.
scm-record yields a selected change with `FileMode::Absent` in one of
two cases:
- when an existing file is deleted and this change is selected,
- when a new file is created and this change is not selected.
From the information provided by `File::get_selected_contents()`
alone, it is not possible to distinguish these two cases. In the first
case, the tree definitely needs to change to reflect the deletion, so
it was marked for deletion. In the second case, that deletion marker
("tombstone") usually was just ignored because no such file existed.
However, when the tombstone happens to coindice with a deleted directory
and a new file has been created in its place, but neither change is
selected, then the tombstone had the effect of deleting the directory
from the tree.
This is now fixed by only marking the file for deletion if a
corresponding file mode change to `Absent` has been "checked".
Otherwise, if the file mode change for the creation of a new file is not
"checked", the file can be merely skipped.
The original form of `create_tree()` is limited to creating (valid
UTF-8) text files but cannot create binary files, executable
files, or symlinks. Dedicated helpers like `write_executable_file()` or
`write_symlink()` partially compensated for this, but required manually
assembling the tree in the test code.
This commit introduces `TestTreeBuilder` which provides an API to
successively add entries to a tree which can represent all of the above.
`TestTreeBuilder` can then create either a single `Tree`, or a resolved
`MergedTree`.
In addition to using `TestTreeBuilder` directly, `create_tree_with()`
and `create_single_tree_with()` accept a closure which receives a
`TestTreeBuilder`. This allows test code to quickly describe the tree
without requiring the a named builder at caller scope. Riffing off
the familiar function names should help in discovering the new builder
facilities. However, it is completely possible to use `TestTreeBuilder`
directly, if preferred.
While the existing test checks that the schema defaults are consistent
with the output of `jj config get` (with default config), it would not
find type errors like the ones fixed in e9da94e.
This add another test case which validates the synthetic TOML containing
the default values according to the schema against the schema itself.
Because the datatests don't use the default libtest harness, datatests
and normal tests cannot be mixed in the same module or even binary.
`test_config_schema` is renamed, both to reflect that it is unlike the
other `test_*` modules, but also to make the name available for "normal"
tests related to the config schema.
Extracts the `"default"` values from the schema and creates a synthetic
TOML file holding all the defaults according to the schema. This is done
through some `jq` magic and is not perfect but rather a best effort.
If `jq` is not available, the test is skipped; in CI `jq` is required.
The test then run `jj config get` in the test env for each key in that
defaults file and compares the resulting value with the schema default.
For a few keys, there are actually no defaults known to `jj config get`,
because they are hard-coded or dynamic. These exceptions are intercepted
and explained in the test.
#6475 changed the default value for `git.write-change-id-header` to
`true` but this was not mirrored in the schema's default, causing the
new `test_config_get_yields_values_consistent_with_schema_defaults` to
fail.
The default value became definitely outdated when "branches" were renamed
to "bookmarks" in 0.20 (and broke in 0.28).
The actual default of `revset-aliases."immutable_heads()"` is in fact
`builtin_immutable_heads()`, now. However, if one were to want override
to default `immutable_heads()`, `builtin_immutable_heads()` would likely
not be a helpful starting point. Therefore, it may make sense to keep
this "resolved" default value in the schema.
As the discussion in #3419 evolved, the old behavior for `jj split` was
enabled by default again in 2af5b60d32, but the default value in the
schema was missed.
I had a report from a user at Google who was confused about a config
deprecation message because they had forgotten that they set a
repo-level config. This patch includes the source level in the warning
message. Hopefully that's sufficient. We could of course print the
path too, but that would make the message much longer so we would have
to split it up on two lines and I'm not sure it's worth it.
