Skip to content

Add prune integration tests for many edge cases#2844

Merged
MichaelEischer merged 1 commit intorestic:masterfrom
aawsome:prune-integration-tests
Oct 6, 2020
Merged

Add prune integration tests for many edge cases#2844
MichaelEischer merged 1 commit intorestic:masterfrom
aawsome:prune-integration-tests

Conversation

@aawsome
Copy link
Copy Markdown
Contributor

@aawsome aawsome commented Jul 19, 2020

What is the purpose of this change? What does it change?

Add prune integration tests for many edge cases like duplicate blobs, mixed blobs or all types of corrupt repo.

Was the change discussed in an issue or in the forum before?

This is part of #2718 that can be merged independently.

Checklist

  • I have read the Contribution Guidelines
  • I have enabled maintainer edits for this PR
  • I have added tests for all changes in this PR
  • I have not added documentation for the changes (in the manual)
  • There's a new file in changelog/unreleased/ that describes the changes for our users (template here)
  • I have run gofmt on the code in all commits
  • All commit messages are formatted in the same style as the other commits in the repo
  • I'm done, this Pull Request is ready for review

@aawsome aawsome mentioned this pull request Jul 19, 2020
11 tasks
@aawsome aawsome force-pushed the prune-integration-tests branch from efe0453 to a18652f Compare July 22, 2020 18:02
@aawsome
Copy link
Copy Markdown
Contributor Author

aawsome commented Jul 22, 2020

Just realized that the "failing test" (it was a blob existing in the pack file but missing in the index) is in fact cured by "old" prune. So I changed this accordingly here.

@aawsome aawsome force-pushed the prune-integration-tests branch from a18652f to 1c80707 Compare August 23, 2020 08:46
@aawsome
Copy link
Copy Markdown
Contributor Author

aawsome commented Aug 23, 2020

rebased after #2674 has been merged.

@J0WI J0WI mentioned this pull request Sep 21, 2020
6 tasks
Copy link
Copy Markdown
Member

@MichaelEischer MichaelEischer left a comment

Choose a reason for hiding this comment

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

How did you generate the test data sets? I wonder if there's an easy way to generate those on the fly?

@aawsome
Copy link
Copy Markdown
Contributor Author

aawsome commented Oct 5, 2020

How did you generate the test data sets? I wonder if there's an easy way to generate those on the fly?

Honestly, by playing a lot around with creating backups, deleting manually single files from the repo, running prune and rebuild-index and sometimes even changing the source code to get the desired test repository.
I don't feel like I'm able to code that with small effort.

Also I think that keeping the repos as tar files "hardens" the tests cases in the sense that future code changes will not spoil the test cases.
E.g. the code-change that removed mixed pack creation effectively prevents generating those test settings on the fly... Similar things could happen with the other cases in future.

@aawsome aawsome force-pushed the prune-integration-tests branch from 1c80707 to b60368e Compare October 5, 2020 20:47
Copy link
Copy Markdown
Member

@MichaelEischer MichaelEischer left a comment

Choose a reason for hiding this comment

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

Also I think that keeping the repos as tar files "hardens" the tests cases in the sense that future code changes will not spoil the test cases.

That will also in parts verify that we don't break repository backwards compatibility.

I'd like to have the edge case tests as subtests to allow selecting a specific one. Besides that the code is ok. (The travis build error will disappear on the next CI run.)

@aawsome aawsome force-pushed the prune-integration-tests branch from b60368e to 6822a58 Compare October 6, 2020 18:20
Copy link
Copy Markdown
Member

@MichaelEischer MichaelEischer left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks :-)

@MichaelEischer MichaelEischer merged commit 0ae02f3 into restic:master Oct 6, 2020
@aawsome aawsome deleted the prune-integration-tests branch November 6, 2020 20:51
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.

2 participants