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

[#2145] Dockerisation of RepoSense #2149

Closed
wants to merge 6 commits into from

Conversation

asdfghjkxd
Copy link
Contributor

@asdfghjkxd asdfghjkxd commented Mar 9, 2024

Part of #2145

Proposed commit message

Currently, there are some issues with regard to consistent testing of
code, especially for the frontend code, due to the difference in
environments where the test cases are run (developer's machine VS
Github Action's workers). This inconsistency in testing results makes
it hard for us to really distinguish false negatives/false positives.

Moreover, this inconsistency in the environment where RepoSense is
installed may also result in dependency issues.

In this PR, a proof-of-concept Dockerfile and docker-compose.yml file
will be provided first to test the viability of such an integration
into RepoSense. Future implementations would see further integration of
Docker into Github Actions, more complicated Docker build processes and
tests and documentation on how to maintain these Docker files and
builds.

Let's move to integrate Docker and Docker Compose into RepoSense to
mitigate and potentially solve some of the aforementioned issues, by
providing users and developers a more standardised environment where
they can run or test RepoSense.

Other information

This PR is part of the following steps outlined in the parent issue:

  • Create Ubuntu Dockerfiles
  • Integrate Ubuntu Dockerfiles with Github Actions

For this PR, only Ubuntu/Linux images are explored, as the rest are in development. Ubuntu/Linux images are also easier to work with and far less fussy compared to Windows and MacOS images, hence starting work on Ubuntu/Linux might be a helpful proof of concept for the viability of Docker for use for testing.

@asdfghjkxd asdfghjkxd requested a review from a team March 15, 2024 13:50
@asdfghjkxd
Copy link
Contributor Author

Do note that the Docker containers currently do not conduct frontend tests using cypress, even though the dependencies are installed on the base Ubuntu containers, as they are consistently failing CI. Will look into the cause and update this thread once I have isolated the cause of the error.

@asdfghjkxd asdfghjkxd marked this pull request as ready for review March 16, 2024 16:27
Copy link
Contributor

@sopa301 sopa301 left a comment

Choose a reason for hiding this comment

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

LGTM! We just need to fix the Cypress issues.

FROM asdfghjklxd/reposense-images:ubuntu-22-04 AS setup-env

LABEL os=ubuntu
LABEL "os.version"="20.04"
Copy link
Contributor

Choose a reason for hiding this comment

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

This should probably be 22.04.

Suggested change
LABEL "os.version"="20.04"
LABEL "os.version"="22.04"

- name: Build Docker Image
run: |
docker build -t ${{ matrix.tag }} -f ${{ matrix.dockerfile }} -o ${{ env.OUTDIR }} .
# - name: Upload Code Coverage (Java 8)
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this going to be re-added?

Copy link
Contributor

Choose a reason for hiding this comment

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

As discussed, since we're not using this for the CI, we can probably put this in a folder marked for local testing.

@asdfghjkxd
Copy link
Contributor Author

As mentioned in issue #2145, we are not pursuing this idea, hence I am closing out this PR.

@asdfghjkxd asdfghjkxd closed this Mar 18, 2024
Copy link
Contributor

The following links are for previewing this pull request:

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

Successfully merging this pull request may close these issues.

None yet

2 participants