Skip to content
This repository was archived by the owner on Oct 10, 2020. It is now read-only.

build: "all" should be default, don't run tests on install #19

Merged
merged 1 commit into from
Feb 18, 2015

Conversation

cgwalters
Copy link
Member

Neither "make" or "make install" should not run tests. Further,
change the default target to be "all" by listing it first.

Neither "make" or "make install" should not run tests.  Further,
change the default target to be "all" by listing it first.
@lsm5
Copy link
Contributor

lsm5 commented Feb 18, 2015

LGTM, tests aren't run in rpmbuild by default with this update.

@cgwalters
Copy link
Member Author

That's what %check is for, no? And we can't run the tests anyways in koji/mock/rpmbuild as they require docker which requires root...

@lsm5
Copy link
Contributor

lsm5 commented Feb 18, 2015

yup, the test could be placed in %check but docker and its root requirement would pose a problem

cgwalters added a commit that referenced this pull request Feb 18, 2015
build: "all" should be default, don't run tests on install
@cgwalters cgwalters merged commit 2c54877 into projectatomic:master Feb 18, 2015
@cgwalters cgwalters deleted the make-fixes branch February 18, 2015 22:27
@rhatdan
Copy link
Member

rhatdan commented Feb 19, 2015

We can do it like we do the docker test, so that we only do the check if the docker socket is available.

@rhatdan
Copy link
Member

rhatdan commented Feb 19, 2015

Then we can run the tests in Jenkens.

@lsm5
Copy link
Contributor

lsm5 commented Feb 19, 2015

@rhatdan hmm, I guess as long as mock doesn't pull in docker given that it's a runtime requirement and trigger tests, we're probably fine. I'll give this a shot and get back

@cgwalters
Copy link
Member Author

How about a new target make rootcheck or something?

@cgwalters
Copy link
Member Author

(Because we could have tests that run as non-root, e.g. internal mocked unit tests and the like)

@lsm5
Copy link
Contributor

lsm5 commented Feb 19, 2015

haven't tried in mock yet, but on a non-mock build, make test did show me some errors:

atomic_test
PID   USER     COMMAND
    1 root     /bin/sh
    9 root     ps
Error response from daemon: Conflict, cannot delete 9dd6ed7c29a4 because the container 3152734b6250 is using it, use -f to force
FATA[0000] Error: failed to remove one or more images

Also:

Step 0 : FROM busybox
 ---> 4986bf8c1536
Step 1 : LABEL 
# Skipping unknown instruction LABEL
 ---> 4986bf8c1536
Step 2 : LABEL 
# Skipping unknown instruction LABEL
 ---> 4986bf8c1536
Step 3 : LABEL 
# Skipping unknown instruction LABEL
 ---> 4986bf8c1536

@lsm5
Copy link
Contributor

lsm5 commented Feb 19, 2015

so running tests in check works fine, mock doesn't complain either.

scratch build results: http://koji.fedoraproject.org/koji/taskinfo?taskID=9000034
updated spec: https://github.com/lsm5/atomic-rpm/blob/master/atomic.spec

@rhatdan
Copy link
Member

rhatdan commented Feb 19, 2015

LABEL would currently require the LABEL patch, which is only in RHEL7 at the moment.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants