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

Improve build system; add e2e tests, docker CD, FAKE, paket #108

Closed
wants to merge 14 commits into from

Conversation

tomzo
Copy link
Contributor

@tomzo tomzo commented Oct 4, 2018

What does this PR do?

  • This PR contains a rebase and some fixes of Improved development environment #101 . @solidsloth I hope you are fine with me rebasing your commit and integrating with FAKE. In Improved development environment #101 quite a few things did not want to work at the same time. You could either get debugging work, tests or production build. I have taken care of this now.
  • Added test which checks that built image returns contents of SPA site
  • It includes all of [WIP] First things for maturity and CD setup; IDE, Paket, FAKE #103, which is:
    1. Adds Idefile, which is a reference to docker IDE image, which contains all build dependencies (dotnet, node.js, yarn, bats for e2e tests). If you have local docker daemon then you can run any of the build tasks like this now ide <command>. E.g. ide ./build.sh will run msbuild publish and run tests with xunit.
    2. Updates travis setup to run:
      • msbuild publish in release config
      • run tests of the published artifacts
      • build docker image from those artifacts
      • test that docker image is functional
    3. Moves dependency management to paket.
    4. Adds FAKE and targets to run most of above workflow.
  • It includes all of Docker the right way #105, which is:
    • It changes the way to build docker image. By always using the result of msbuild /t:Publish in the docker image. And we are not using docker BUILD container, but using the same artifacts that were tested in previous stage.
    • added a flag to skip running db migrations. By default it is ran. As mentioned in discussion in Improved development environment #101
    • added e2e tests, mostly ported from LiGet.
      • We create some packages, push, download with nuget cli and paket.
      • These can run on travis from now on

Dockerfile is changed a bit:

  • uses tini for init system to reap any processes.
  • does not run as root by default. There is a baget user.
  • because of above changes default listening port to 9090.
  • when container starts it changes uid and gid of baget user based on owner of volume mount.
  • environment variables are all specified in Dockefile for clarity. Now a basic server to try out can be run with docker run baget.

Closes Issue(s)

#57 #56 #101 #7

Motivation

In general - I need BaGet to be functional and tested before I can use it in production. I don't want to waste time on testing features and fixing bugs which get randomly broken because there are no tests, no reproducible builds and no release cycle.

Why Idefile and IDE project:

  • biggest advantage is to reduce the noise around works on my machine. Now anyone can download the same environment which all developers and travis agents are running for builds.
  • rather than spending time on setting up your host, (either laptop or the CI agent) just pull the image which someone has already created and tested to be sufficient to execute entire workflow of the project

Why paket:

  • paket gets consistent package versions across all projects and it can support modules for FAKE.
  • paket.dependencies gives you an quick overview of all packages used by application.
  • paket.lock gives you detailed info about each version of all direct and transitive dependencies.
  • I tried to install xunit.runner.console without using paket. But there is no such command on dotnet SDK. There is no nuget CLI for linux which works without mono, and I managed to keep away from mono so far.

Why FAKE:

  • we get consistent AssemblyVersion for all projects, from top of changelog file.
  • it provides single source of truth on what the release cycle and tasks involved.
  • fake is cross platform and can do much more than just msbuild. (See how hard it was to copy a few files in Improved development environment #101)
  • programming tasks in xml is not easy and error prone. In F# FAKE you get as much as type safety in build scripts.
  • for all other task, which cannot be covered by msbuild or dotnet cli, we need a cross platform language. Otherwise windows guys start to code in powershell and linux guys in bash. We need something friendly to both platforms.
  • F# is cool

Why I am running tests with dotnet ...xunit.console.dll <assemblies-from-publish-dir>?
The main point in CD is that artifacts which constitute the application should be produced just once and then ran through all QA stages. We really need to be testing exactly the same files at each point until final release to ensure quality. In dotnet world that means using dotnet publish to force outputting application and all all its dependencies.
One of the common issues I had with dotnet assemlies is that running with dotnet test adapter uses additional assemblies from nuget caches. So I've seen tests passing when ran with dotnet test or dotnet xunit, but when same test is executed with dotnet ...xunit.console.dll <assemblies-from-publish-dir> it fails. Naturally same load errors happen in deployed application (after all tests have passed earlier...). So running tests with publish reproduces those errors and makes sure that we get load errors early.
PS: I actually ran into this already in BaGet and fixed it. The Microsoft.AspNetCore.App in BaGet.Tests was causing problems like this:

xUnit.net Console Runner v2.4.0 (64-bit .NET Core 4.6.26814.03)
System.IO.FileNotFoundException: Could not load file or assembly 'Microsoft.Extensions.Logging.Abstractions, Version=2.1.1.0, Culture=neutral, PublicKeyToken=adb9793829ddae60'. The system cannot find the file specified.

System.IO.FileNotFoundException: Could not load file or assembly 'Microsoft.Extensions.Logging.Abstractions, Version=2.1.1.0, Culture=neutral, PublicKeyToken=adb9793829ddae60'. The system cannot find the file specified.

Additional Notes

I will be away for weekend, so I can have a look at review notes on Monday.
I am closing #103 and #105 , and I hope @solidsloth can close #101 since this contains the work and original intentions.

tomzo added 14 commits October 4, 2018 19:03
This should provide sufficient environment for building and testing on
linux. Includes debian and alpine based images
For usage see https://github.com/ai-traders/docker-dotnet-ide
In development the application can now be run to proxy to the react
development server. This resolves the problem where the code needs to be
changed in development to point to the server correcly (which is running
on a different port). I also modified the csproj file to have msbuild
copy the required front-end files to the publish folder when needed.
Therefore the built files no longer need to be copied over and
permanently stored in the wwwroot because in development the requests
are proxied, and in production the files are copied to the publish
folder.
- remove the target in msbuild, which has ugly workarounds for even such
a simple job as copying some files.
- we can still use msbuild in debug config to rebuild spa, which
convenient for quick iterations
- use FAKE tasks to create final SPA site and put it in publish
directory.
- added e2e test that validates that SPA is returned by built docker
image
- updates IDE docker image to include node.js and yarn
@loic-sharma
Copy link
Owner

loic-sharma commented Nov 4, 2018

Hey @tomzo, I apologize for the incredibly long response time. I feel conflicted with these changes as I want to keep BaGet as simple as possible.

IDE project:
While I see the value in a reproducible development environment, I don't feel comfortable taking a dependency on this project. I'm not particularly familiar with IDE, and I'd rather focus my time on the features that differentiate BaGet. I don't think that setting up a development environment to work on BaGet is too difficult. I'd like to keep it that way.

Paket:
Paket is a great project. However, I work on NuGet and would like to stick to NuGet 😄

paket gets consistent package versions across all projects
paket.dependencies gives you an quick overview of all packages used by application.

This is possible using NuGet too (see the ASP.NET Core repository for an example). NuGet will be improving this experience in the future, see this spec: Centrally managing NuGet package versions. The paket.dependencies file is indeed pretty neat!

paket.lock gives you detailed info about each version of all direct and transitive dependencies.

Yes, this a feature that NuGet is sorely lacking. We are actively working on this, see this announcement.

There is no nuget CLI for linux which works without mono, and I managed to keep away from mono so far.

Out of curiosity, why do you want to use the nuget CLI on linux? The dotnet CLI should have most everything you need.

FAKE:

fake ... can do much more than just msbuild. (See how hard it was to copy a few files in #101)
programming tasks in xml is not easy and error prone. In F# FAKE you get as much as type safety in build scripts.

I 110% agree. I'm very torn about this as I very much dislike MSBuild. However, I want to keep BaGet as approachable as possible to all .NET developers. I don't think FAKE fits well with this goal.

for all other task, which cannot be covered by msbuild or dotnet cli, we need a cross platform language. Otherwise windows guys start to code in powershell and linux guys in bash. We need something friendly to both platforms.

I agree that we should avoid this. If possible, I'd like to keep all tasks in MSBuild (😢). Is there anything in particular that you think would be too crazy for MSBuild?

Okay, I'm gonna stop here as I'd love to hear your thoughts @tomzo. I realize that I haven't addressed your test and Docker changes. I'm much more onboard with these changes, so that should be smoother :)

@tomzo
Copy link
Contributor Author

tomzo commented Nov 4, 2018

Paket ... This is possible using NuGet too

Without paket.lock the build is not reproducible, because of transitive dependencies not being locked. Which is not acceptable for me, for any project which I intend to deploy. Maybe NuGet will be capable of this one day, that's great. But a solution now is paket.

Out of curiosity, why do you want to use the nuget CLI on linux? The dotnet CLI should have most everything you need.

nuget cli has install command which works as downloader of nupkg. Other than I don't need it.
I had to download xunit console runner, to run tests as I explained earlier. This helps to reproduce assembly load errors such as #112 during tests.

I agree that we should avoid this. If possible, I'd like to keep all tasks in MSBuild (cry). Is there anything in particular that you think would be too crazy for MSBuild?

I don't see a problem in running msbuild for restore, build and test. Because those tasks are trivial.
Have a look all tasks I implemented in https://github.com/ai-traders/BaGet/blob/master/build.fsx for managing release cycle, changelog, git tags, publishing to github. Do you really intend to have those in msbuild?

Okay, I'm gonna stop here as I'd love to hear your thoughts @tomzo. I realize that I haven't addressed your test and Docker changes. I'm much more onboard with these changes, so that should be smoother :)

But you are saying that we cannot use FAKE, paket and IDE. Each of those is critical to make those end tests work. Sorry, but I won't be rebasing those tests without any framework, because it would be a lot of wasted time on setups, which would only poorly mimic the tools needed to get the job done.

My initial intent was to replace LiGet by merging most of it into BaGet and then move to it. I have mostly done it already in my fork. But now it looks to me that we have different objectives about nuget server project.
You want to keep it as simple as possible and easy for (I guess) Visual Studio developers. I think that you are also OK with this being still an experimental, PoC project. I need a working, maintainable product, which BaGet is not, because it is not tested and released.
So in my opinion, the best to do here is split ways again. I already have 1/2 of LiGet in BaGet fork, so I will just port it back to LiGet and keep my build system, tests and releases, which I had from LiGet anyway.
I think we agree on adding more tests coverage, so please feel free to cherry-pick some of tests which I added from my fork - https://github.com/ai-traders/BaGet/tree/master/tests/BaGet.Tests

@tomzo tomzo closed this Nov 4, 2018
@BlythMeister
Copy link

As someone external, I hope that we can find some middle ground on this.
Whilst I am a huge advocate of paket and fake (and a contributor to both) I can see why there would be apprehension to add to this project.

The 2 things I think this project is missing is the end to end tests to prove it works the same as nuget.

The ability for someone to pick up a docker image, or package to host themselves.
For me, I don't want to host I docker, but spent around an hour trying to get something which I could self host.

My suggestion is to work out how to add these tests and other changes which add value to the project, and then investigate other build tools for example appveyor or Azure pipelines.
Which both support the ability to create things like GitHub releases without the need for fake.

@loic-sharma
Copy link
Owner

loic-sharma commented Nov 4, 2018

I need a working, maintainable product, which BaGet is not, because it is not tested and released.
So in my opinion, the best to do here is split ways again.

That's completely fair. I'd like to investigate @BlythMeister's suggestion of Azure pipelines for releases.

I think we agree on adding more tests coverage, so please feel free to cherry-pick some of tests which I added from my fork - https://github.com/ai-traders/BaGet/tree/master/tests/BaGet.Tests

Will do . Thank you for the great work.

The ability for someone to pick up a docker image, or package to host themselves.
For me, I don't want to host I docker, but spent around an hour trying to get something which I could self host.

This sounds interesting @BlythMeister . Do you know of other projects that support self hosting through packages? I'd love to mimic their setup.

@tomzo If BaGet had tests and was packaged into NuGet packages, would this be a good middleground?

@BlythMeister
Copy link

BlythMeister commented Nov 4, 2018

I don't know of any...but if it helps, I ran a dotnet publish and then got the output.
That as a zip in a GitHub release would be like gold dust!

An Azure pipelines might even be able to publish that into GitHub...and maybe push the docker image into docker hub...then your totally cooking on gas.

I played a but with pipelines, they look powerful.
But since this project is all dotnet core, I think having a small pipeline to restore, build, publish/package and upload artifacts (docker image and binaries) into relevent artifact stores would turn this from a useful but very manual project into something truly awesome.

Also, some useful quick start guides (happy to write one for self hosting since that's my approach using iis as a reverse proxy)

@loic-sharma
Copy link
Owner

I've added Continuous Delivery to BaGet. You can:

I'll add support for Docker images in the future. Please let me know what you think!

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.

3 participants