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

mail-client/neomutt: Add unit tests #16886

Closed
wants to merge 2 commits into from

Conversation

nicolasbock
Copy link
Member

This change addds unit tests to neomutt. Those tests come in a separate
package as upstream maintains them in a separate repository.

Closes: https://bugs.gentoo.org/show_bug.cgi?id=734122
Signed-off-by: Nicolas Bock nicolasbock@gentoo.org

@gentoo-bot
Copy link

Pull Request assignment

Submitter: @nicolasbock
Areas affected: ebuilds
Packages affected: mail-client/neomutt, mail-client/neomutt-test-files

mail-client/neomutt: @nicolasbock
mail-client/neomutt-test-files: @gentoo/proxy-maint (new package)

Linked bugs

Bugs linked: 734122


In order to force reassignment and/or bug reference scan, please append [please reassign] to the pull request title.

Docs: Code of ConductCopyright policy (expl.) ● DevmanualGitHub PRsProxy-maint guide

@gentoo-bot gentoo-bot added new package The PR is adding a new package. self-maintained The PR changes only packages that are maintained by the submitter (i.e. no need to ask anybody else) assigned PR successfully assigned to the package maintainer(s). bug linked Bug/Closes found in footer, and cross-linked with the PR. labels Jul 28, 2020
@nicolasbock nicolasbock force-pushed the neomutt-tests branch 4 times, most recently from bde6a09 to 605f853 Compare July 29, 2020 13:05
Comment on lines 15 to 19
IUSE=""

DEPEND=""
RDEPEND="${DEPEND}"
BDEPEND=""
Copy link
Member

Choose a reason for hiding this comment

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

These are all just waste of space in the ebuild and can be removed if you wish so.

Copy link
Member Author

Choose a reason for hiding this comment

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

Done.

Comment on lines 21 to 23
src_unpack() {
if [[ -n ${A} ]]; then
unpack ${A}
fi
mv "${WORKDIR}/${PN}-${UPSTREAM_COMMIT}" "${WORKDIR}/${P}"
}
Copy link
Member

Choose a reason for hiding this comment

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

You can just define S="..." before any src_ phases, making all this redundant as well.

Copy link
Member Author

Choose a reason for hiding this comment

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

Done


src_prepare() {
eapply_user
NEOMUTT_TEST_DIR="${S}" ./setup.sh
Copy link
Member

Choose a reason for hiding this comment

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

External commands need to be appended with || die.

Copy link
Member Author

Choose a reason for hiding this comment

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

Done


src_install() {
dodir /usr/share/${P}
cp --archive "${S}" /usr/share
Copy link
Member

Choose a reason for hiding this comment

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

Here too, || die.

Copy link
Member Author

Choose a reason for hiding this comment

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

Done

@nicolasbock nicolasbock force-pushed the neomutt-tests branch 4 times, most recently from 5e00bf1 to 4cca854 Compare July 29, 2020 16:04
@thesamesam thesamesam self-requested a review August 1, 2020 01:27
Copy link
Member

@thesamesam thesamesam left a comment

Choose a reason for hiding this comment

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

I get this (snippet):


./mbox/
./mbox/apple.mbox
tar: ./mbox/endive.mbox: Cannot open: Permission denied
./mbox/fig.mbox
./mbox/damson.mbox/
./mbox/damson.mbox/.gitignore
./mbox/symlink
./mbox/banana.mbox
./mbox/cherry.mbox
tar: Exiting with failure status due to previous errors
 �[31;01m*�[0m ERROR: mail-client/neomutt-test-files-20200626::gentoo failed (prepare phase):
 �[31;01m*�[0m   Failed to tar the test files

neomutt-test-files.log

)

econf CCACHE=none "${myconf[@]}"
}

src_test() {
local test_dir=$(mktemp --directory)
tar -C ${test_dir} -xvf /usr/share/neomutt-test-files-${PV}/neomutt-test-files-${PV}.tar.gz
Copy link
Member

@thesamesam thesamesam Aug 1, 2020

Choose a reason for hiding this comment

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

|| die

local test_dir=$(mktemp --directory)
tar -C ${test_dir} -xvf /usr/share/neomutt-test-files-${PV}/neomutt-test-files-${PV}.tar.gz
NEOMUTT_TEST_DIR=${test_dir} emake test
rm -rf ${test_dir}
Copy link
Member

@thesamesam thesamesam Aug 1, 2020

Choose a reason for hiding this comment

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

|| die

Copy link
Member Author

Choose a reason for hiding this comment

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

Fixed

)

econf CCACHE=none "${myconf[@]}"
}

src_test() {
local test_dir=$(mktemp --directory)
Copy link
Member

@thesamesam thesamesam Aug 1, 2020

Choose a reason for hiding this comment

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

I think we may need to either put || die inside the subshell (ok on >= EAPI 7)

... or check if it's a directory with a Bash test ([[ -d ... ]]) afterwards, and die if it isn't.

Copy link
Member Author

Choose a reason for hiding this comment

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

Fixed

@nicolasbock nicolasbock force-pushed the neomutt-tests branch 2 times, most recently from 5026956 to 0aa3d97 Compare August 22, 2020 20:22
@nicolasbock
Copy link
Member Author

@juippis and @thesamesam Sorry this revision took a little. Could you have another look please? Thanks!

@nicolasbock nicolasbock force-pushed the neomutt-tests branch 2 times, most recently from d4ab0b7 to 0d84c19 Compare August 22, 2020 20:53
@gentoo-repo-qa-bot
Copy link
Collaborator

Pull request CI report

Report generated at: 2020-08-22 21:01 UTC
Newest commit scanned: d4ab0b7
Status: ✅ good

There are existing issues already. Please look into the report to make sure none of them affect the packages in question:
https://qa-reports.gentoo.org/output/gentoo-ci/6d7a080e7a/output.html

@nicolasbock nicolasbock force-pushed the neomutt-tests branch 2 times, most recently from 7752888 to fe2d556 Compare August 22, 2020 21:08
@gentoo-repo-qa-bot
Copy link
Collaborator

Pull request CI report

Report generated at: 2020-08-22 21:21 UTC
Newest commit scanned: 7752888
Status: ✅ good

There are existing issues already. Please look into the report to make sure none of them affect the packages in question:
https://qa-reports.gentoo.org/output/gentoo-ci/900b5fd3e9/output.html

@gentoo-repo-qa-bot
Copy link
Collaborator

Pull request CI report

Report generated at: 2020-08-22 21:41 UTC
Newest commit scanned: fe2d556
Status: ✅ good

There are existing issues already. Please look into the report to make sure none of them affect the packages in question:
https://qa-reports.gentoo.org/output/gentoo-ci/07d608b81d/output.html

@@ -0,0 +1 @@
DIST neomutt-test-files-1.tar.gz 3673 BLAKE2B 50354f19aedc5fc07d59ecb5b38fa65de16119bc0929f47014bd35dab4d3a4e5953c8b35670f3440003cf0cbccc2d0d2b3a869dc929cdc3cd60b02790270fb54 SHA512 3d4962210bc558234d818801dcaa7851a0aef011c96d91c054af535186ffda42059fc61fb148c48e1b076999fe3159b31589a69a29ed1897f8928f52fcc157d3
Copy link
Member

Choose a reason for hiding this comment

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

We have rules how to name packages such as this,
https://devmanual.gentoo.org/ebuild-writing/file-format/index.html#snapshots-and-live-ebuilds

neomutt-test-files-0_pre20200618.ebuild
So when -1.0 is released, it's going to be higher than the snapshot version.

Copy link
Member Author

Choose a reason for hiding this comment

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

Fixed

Comment on lines 19 to 31
src_unpack() {
if [[ -n ${A} ]]; then
mkdir --verbose -p "${S}" || die "Could not create ${S}"
fi
}

src_prepare() {
if [[ -n ${A} ]]; then
cd "${DISTDIR}" || die "Could not cd into DISTDIR"
cp --verbose ${A[@]} "${S}" || die "Could not copy package"
fi
eapply_user
}
Copy link
Member

Choose a reason for hiding this comment

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

All of this seems rather redundant to me. It should just work if you define S, as you've done?

Rest look ok!

Copy link
Member Author

Choose a reason for hiding this comment

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

Maybe. I don't care about unpacking the source though. I would like to package the tar file so that I can unpack it during the testing phase of the neomutt package. Defining S would not help there. Or am I missing something?

Copy link
Member

Choose a reason for hiding this comment

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

Okay, would it then be possible to just do this in the neomutt ebuild? If I understood correctly, the contents of this package is just required on neomutt for testing phase, and not really needed to be installed on / to run the tests?

Copy link
Member Author

Choose a reason for hiding this comment

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

Well, that's a good point 😄 I hadn't thought of that. Please have a look at the updated PR.

@nicolasbock nicolasbock force-pushed the neomutt-tests branch 2 times, most recently from 2e32ad9 to 0378824 Compare August 25, 2020 17:33
@gentoo-repo-qa-bot
Copy link
Collaborator

Pull request CI report

Report generated at: 2020-08-25 17:50 UTC
Newest commit scanned: 0378824
Status: ✅ good

There are existing issues already. Please look into the report to make sure none of them affect the packages in question:
https://qa-reports.gentoo.org/output/gentoo-ci/512af81846/output.html

Copy link
Member

@thesamesam thesamesam left a comment

Choose a reason for hiding this comment

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

Pop the version number in 'version bump' commits (like version bump to N) and it's looking great to me. All tests are passing now too! 💯

pushd ${test_dir} || die "Could not cd into test_dir"
NEOMUTT_TEST_DIR="${test_dir}" ./setup.sh \
|| die "Failed to run the setup.sh script"
popd
Copy link
Member

Choose a reason for hiding this comment

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

|| die

Copy link
Member Author

Choose a reason for hiding this comment

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

fixed

!slang? ( sys-libs/ncurses:0 )
slang? ( sys-libs/slang )
ssl? (
!libressl? ( >=dev-libs/openssl-0.9.6:0 )
Copy link
Member

Choose a reason for hiding this comment

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

We link against openssl/libressl and they have a subslot, so we should use := on these two to make sure we get rebuilt if they break ABI compatibility

kerberos? ( virtual/krb5 )
notmuch? ( net-mail/notmuch )
sasl? ( >=dev-libs/cyrus-sasl-2 )
!slang? ( sys-libs/ncurses:0 )
Copy link
Member

Choose a reason for hiding this comment

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

Link against ncurses and it has a subslot (check ebuild and see if it has a SOMETHING/N as SLOT, where N is the subslot -- it's ok if SOMETHING is 0), so let's subscribe to it: :=

I'll let you check the others to see if we need more subscriptions! 😃

Copy link
Member Author

Choose a reason for hiding this comment

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

Done


EAPI=7

inherit eutils flag-o-matic
Copy link
Member

Choose a reason for hiding this comment

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

When I skimmed, I wasn't sure if we were using flag-o-matic anymore

Copy link
Member Author

Choose a reason for hiding this comment

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

done

This change addds unit tests to neomutt. Those tests come from a
separate repository.

Closes: https://bugs.gentoo.org/734122
Closes: https://bugs.gentoo.org/728886
Signed-off-by: Nicolas Bock <nicolasbock@gentoo.org>
Signed-off-by: Nicolas Bock <nicolasbock@gentoo.org>
@gentoo-repo-qa-bot
Copy link
Collaborator

Pull request CI report

Report generated at: 2020-08-25 20:10 UTC
Newest commit scanned: 1732db8
Status: ✅ good

There are existing issues already. Please look into the report to make sure none of them affect the packages in question:
https://qa-reports.gentoo.org/output/gentoo-ci/fbb3f6a36c/output.html

@nicolasbock
Copy link
Member Author

Merged.

@nicolasbock nicolasbock deleted the neomutt-tests branch September 25, 2020 14:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
assigned PR successfully assigned to the package maintainer(s). bug linked Bug/Closes found in footer, and cross-linked with the PR. new package The PR is adding a new package. self-maintained The PR changes only packages that are maintained by the submitter (i.e. no need to ask anybody else)
Projects
None yet
5 participants