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

Run tests inside librecores CI docker image #82

Merged
merged 7 commits into from Jun 8, 2019

Conversation

@Nancy-Chauhan
Copy link
Contributor

commented May 29, 2019

Solves issue #81

@oleg-nenashev

This comment has been minimized.

Copy link

commented May 30, 2019

@Nancy-Chauhan please use mentions to request reviews

@oleg-nenashev
Copy link

left a comment

As we discussed at the call, please make sure that you update/remove Travis scripts so that we can have the same set of scripts for Travis and LibreCores CI

@oleg-nenashev

This comment has been minimized.

Copy link

commented Jun 2, 2019

@stffrdhrn Hi. Did you have a chance to configure CI for your fork?

@stffrdhrn

This comment has been minimized.

Copy link
Member

commented Jun 2, 2019

@oleg-nenashev
Do you mean with Travis? Yes, the link is here, I am not sure if it's public
https://travis-ci.org/stffrdhrn/mor1kx/branches

I haven't tried anything on librecores-ci yet.

@oleg-nenashev
Copy link

left a comment

wrong Docker image

.travis.yml Outdated Show resolved Hide resolved
.travis.yml Outdated
@@ -3,6 +3,9 @@ cache: ccache
sudo: true
dist: trusty

services:
- docker

addons:

This comment has been minimized.

Copy link
@oleg-nenashev

oleg-nenashev Jun 3, 2019

I doubt it is needed after switching to the Docker image

@Nancy-Chauhan

This comment has been minimized.

Copy link
Contributor Author

commented Jun 4, 2019

CC @oleg-nenashev @stffrdhrn Can you please have a look. This seems to work with librecores-ci-image , but fails for verilator job and ESPRESSO pipeline. Log : https://travis-ci.com/Nancy-Chauhan/mor1kx/jobs/205332205

@oleg-nenashev
Copy link

left a comment

The change itself looks good, but it prevents parallel execution of tests. After this change the build will be taking around 50 minutes. Are you fine with that @stffrdhrn ? Or do we need parallelization?

@stffrdhrn

This comment has been minimized.

Copy link
Member

commented Jun 4, 2019

Let's keep it in parallel. Instead of having the test.sh script that runs everything. Just use stages and run a different docker cmd and have different environment variables per run.

@stffrdhrn

This comment has been minimized.

Copy link
Member

commented Jun 4, 2019

@oleg-nenashev @Nancy-Chauhan
In addition to the last comment. The reason we use stages it to have a fall fast pipeline. If verilator checks fail we don't want to go onto the regression testing. Just fail fast.

Also, note we have marked espresso as known to fail.

With the test.sh we can't do either of those.

@Nancy-Chauhan

This comment has been minimized.

Copy link
Contributor Author

commented Jun 7, 2019

@stffrdhrn @oleg-nenashev Can you please check the recent changes
Travis log : https://travis-ci.com/Nancy-Chauhan/mor1kx/builds/114753627

@Nancy-Chauhan Nancy-Chauhan marked this pull request as ready for review Jun 7, 2019

@stffrdhrn

This comment has been minimized.

Copy link
Member

commented Jun 8, 2019

@stffrdhrn @oleg-nenashev Can you please check the recent changes
Travis log : https://travis-ci.com/Nancy-Chauhan/mor1kx/builds/114753627

This looks great! If you are done I can merge this.

@@ -12,7 +12,7 @@
# 'RUN .travis/install-verilator.sh' to this dockerfile to have an image
# with the full environment already installed.
#
FROM ubuntu:trusty
FROM librecores/librecores-ci:latest

This comment has been minimized.

Copy link
@stffrdhrn

stffrdhrn Jun 8, 2019

Member

It we run librecores/librecores-ci image in our tests do we need this .travis/Dockerfile anymore? Do we use this file? If its not needed I suggest we just delete this.

This comment has been minimized.

Copy link
@Nancy-Chauhan

Nancy-Chauhan Jun 8, 2019

Author Contributor

@stffrdhrn This dockerfile does the same work what it used to do before i.e to simulate the travis environment only for testing purposes,this is not actually used in travis . And it works fine with Librecores-ci base image on local machine

This comment has been minimized.

Copy link
@stffrdhrn

stffrdhrn Jun 8, 2019

Member

Yes, but we needed to simulate the Travis environment to have something consistent for running tests. Now the tests run in docker so it's consistent. This is not needed. Anyway I can remove.

@Nancy-Chauhan

This comment has been minimized.

Copy link
Contributor Author

commented Jun 8, 2019

@oleg-nenashev @stffrdhrn I think it is done . But Stafford is the verilator log fine to you ? Because librecores-ci uses the recent version

@stffrdhrn stffrdhrn merged commit c4d1cb0 into openrisc:master Jun 8, 2019

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
@oleg-nenashev

This comment has been minimized.

Copy link

commented Jun 8, 2019

I was a bit late to the party. Thanks @Nancy-Chauhan, it is great to see CI running in the repo again !

@stffrdhrn

This comment has been minimized.

Copy link
Member

commented Jun 8, 2019

I just merged because it's working. We can always do more changes. We probably also want this change on the or1k_marocchino core.

@Nancy-Chauhan

This comment has been minimized.

Copy link
Contributor Author

commented Jun 8, 2019

Yes @stffrdhrn, Can you please give more reference on or1k_marocchino core.

@stffrdhrn

This comment has been minimized.

Copy link
Member

commented Jun 8, 2019

@Nancy-Chauhan

This comment has been minimized.

Copy link
Contributor Author

commented Jun 8, 2019

@oleg-nenashev @stffrdhrn Yes I see it is very similar. Will open PR soon.

@Nancy-Chauhan Nancy-Chauhan deleted the Nancy-Chauhan:lcci branch Jun 9, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.