-
Notifications
You must be signed in to change notification settings - Fork 162
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
Hook up running make test with testport. #355
base: master
Are you sure you want to change the base?
Conversation
Thanks for working on this. Note 'test' needs to be handled by the '-k' mechanism, implied by '-cat', so the error from 'test' doesn't immediately stop but is just noted as a failure in the end. - So plist/qa testing is still done. |
It's going to make the patch a bit bigger 😄 |
d29848c
to
74e1e40
Compare
Is that something like this that you had in mind ? |
Yup, looks familiar (I don't have time to dive into this now). Do we want 'test' to always run? Note also that DEVELOPER is not passed down unless we allow it. Grep for DEVELOPER in common.sh and you'll see you need to do similar where the test framework needs it. We also need to fetch TEST_DEPENDS in |
While I don't have a lot of time on this now, I'll merge what looks right and it can be put into -devel for wider testing. |
test is a no-op if not enabled in the port's Makefile, so I would say yes. I'll have a look at the rest later. |
src/share/poudriere/common.sh
Outdated
@@ -2090,7 +2090,7 @@ _real_build_port() { | |||
# Don't need to install if only making packages and not | |||
# testing. | |||
[ -n "${PORTTESTING}" ] && \ | |||
install_order="${install_order} install-mtree install" | |||
install_order="${install_order} test install-mtree install" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
test runs after install
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well, they don't depend on install, they depend on stage, so, I'm not certain it's true.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh. Stage and install are messed up and install
has different meaning from inside and outside the port. I assume the later is meant here, so the code is correct.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well, maybe tests need to run after the port has been really installed, and not just staged, I don't know, I'm just saying this is coherent with what bsd.port.mk does :-)
Ok, now, it also builds TEST_DEPENDS. |
So, there is another problem, that I'm not quite certain on how to fix, test depends are sometime circular, today, I got a:
I'm not exactly sure on how to fix that. |
ping ? |
You said there was a circular dependency problem. Is that fixed? Regards,
|
Can we maybe only enable tests which have no TEST_DEPENDS as a first step? |
Given that the |
I think what we need to do to fix the cyclic dependency problem is add separate items into the queue. One to build the package, and another to test it. The test one will install the test depends (extract it again), install itself, then handle running the tests. Granted I haven't looked at this in depth but I think this may ultimately be the need. #259 would be needed first. |
If poudriere could detect that adding TEST_DEPENDS doesn't add a cyclic dependency, it could run make test at the same time as the rest, and only postpone make test if there is a cyclic dependency. |
I'm going to make
|
A fix for #259 is nearly ready to merge. I will try to get port testing into 3.2. |
The queue needs to be smarter. Rather than everything in the queue being implicitly "build foo" it needs to be explicit. So "build foo deps bar" and "test foo deps foo baz". This avoids the double pass since the 1 queue can deal with it without reworking all of bulk/testport to deal with 2 passes. |
Can one of the admins verify this patch? |
Ok the whole idea of testing separately is killed because 'make test' depends on 'make stage'. So it must happen at the same time or I need to tar the workdir. So it seems the simplest solution is going to be to drop testing of anything that has a circular dependency. Not sure if tsort will allow that or not. |
ping |
I have this working in a local branch named And for some ports we can likely combine the build and test jobs into 1 without the wrkdir tarring. |
ping |
This is needed to support a 2nd queue for TEST_DEPENDS and smarter job dependencies. Issue #355
The queue work is done. The feature can be implemented now. |
No description provided.