Skip to content

Add pkg_tester #4818

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.

Sign up for GitHub

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

Jump to bottom
Merged
sfc-gh-anoyes merged 12 commits into from
Aug 24, 2021
Merged

Add pkg_tester #4818

sfc-gh-anoyes merged 12 commits into from
Aug 24, 2021

Conversation

@sfc-gh-anoyes
Copy link
Collaborator

@sfc-gh-anoyes sfc-gh-anoyes commented May 13, 2021 •
edited
Loading

Recently we've been having some problems with our packaging (e.g. #4731 ). This PR adds some basic tests we can run to check that our generated rpm and deb packages behave as expected after making packaging changes.

I tested leaking docker images/containers by running the tests and interrupting them with ctrl-c. These tests do a pretty good job, but I added some instructions to the README about what to do just in case.

This change now also incorporates the fixes from #5391, and adds tests for the versioned packages.

Code-Reviewer Section

The general guidelines can be found here.

Please check each of the following things and check all boxes before accepting a PR.

  • The PR has a description, explaining both the problem and the solution.
  • The description mentions which forms of testing were done and the testing seems reasonable.
  • Every function/class/actor that was touched is reasonably well documented.

For Release-Branches

If this PR is made against a release-branch, please also check the following:

  • This change/bugfix is a cherry-pick from the next younger branch (younger release-branch or master if this is the youngest branch)
  • There is a good reason why this PR needs to go into a release branch and this reason is documented (either in the description above or in a linked GitHub issue)

@jzhou77 jzhou77 requested a review from sfc-gh-mpilman May 17, 2021 20:25
sfc-gh-mpilman
sfc-gh-mpilman previously requested changes May 17, 2021
View reviewed changes
Copy link
Collaborator

@sfc-gh-mpilman sfc-gh-mpilman left a comment

Choose a reason for hiding this comment

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

There is already something similar in build/cmake/package_tester -- can you please either:

  • Use that one or
  • Add a note why not and delete the old one?

Having to test systems for this seems to be overkill.

@sfc-gh-anoyes sfc-gh-anoyes marked this pull request as draft May 24, 2021 20:40
@sfc-gh-anoyes sfc-gh-anoyes marked this pull request as ready for review August 19, 2021 16:49
@sfc-gh-anoyes
Copy link
Collaborator Author

sfc-gh-anoyes commented Aug 19, 2021

There is already something similar in build/cmake/package_tester -- can you please either:

  • Use that one or
  • Add a note why not and delete the old one?

Having to test systems for this seems to be overkill.

The build folder has since been deleted so this is no longer relevant.

sfc-gh-abeamon
sfc-gh-abeamon previously approved these changes Aug 19, 2021
View reviewed changes
Copy link
Collaborator

@sfc-gh-abeamon sfc-gh-abeamon 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 reasonable to me, and if I understand correctly from the commit history it seems to have already helped you identify some problems with the packages. I notice there's one comment from Markus about the gitignore file, do you intend to use his suggestion?

sfc-gh-anoyes
sfc-gh-anoyes commented Aug 19, 2021
View reviewed changes
Copy link
Collaborator Author

@sfc-gh-anoyes sfc-gh-anoyes 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 reasonable to me, and if I understand correctly from the commit history it seems to have already helped you identify some problems with the packages. I notice there's one comment from Markus about the gitignore file, do you intend to use his suggestion?

Suggestion taken. PTAL

sfc-gh-abeamon
sfc-gh-abeamon previously approved these changes Aug 19, 2021
View reviewed changes
@sfc-gh-anoyes sfc-gh-anoyes dismissed sfc-gh-mpilman’s stale review August 24, 2021 19:22

This review is no longer relevant since the build/ dir was deleted

@sfc-gh-anoyes sfc-gh-anoyes merged commit 3b4fed4 into apple:master Aug 24, 2021
@sfc-gh-anoyes sfc-gh-anoyes mentioned this pull request Aug 24, 2021
Merged
5 tasks
sfc-gh-anoyes added a commit to sfc-gh-anoyes/foundationdb that referenced this pull request Aug 24, 2021
@sfc-gh-anoyes
Cherry pick apple#4818 to release-7.0
96dafd2
@sfc-gh-abeamon sfc-gh-abeamon mentioned this pull request Aug 25, 2021
Merged
5 tasks
sfc-gh-anoyes added a commit that referenced this pull request Aug 26, 2021
@sfc-gh-anoyes
Merge pull request #5460 from sfc-gh-anoyes/anoyes/pkg_tester-6.3
2cf31ea 
Cherry pick (#4818) - pkg_tester and packaging fixes (release-6.3)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@sfc-gh-abeamon sfc-gh-abeamon sfc-gh-abeamon approved these changes

@sfc-gh-mpilman sfc-gh-mpilman Awaiting requested review from sfc-gh-mpilman

Assignees

@sfc-gh-mpilman sfc-gh-mpilman

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@sfc-gh-anoyes @sfc-gh-mpilman @sfc-gh-abeamon