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

CI tasks for compiler are not running any tests #753

Closed
vogelsgesang opened this issue May 15, 2020 · 5 comments
Closed

CI tasks for compiler are not running any tests #753

vogelsgesang opened this issue May 15, 2020 · 5 comments
Labels

Comments

@vogelsgesang
Copy link
Contributor

It seems that the current CI setup accidentally doesn't run any tests, while still displaying a "passing" state.

Looking at https://circleci.com/gh/kaitai-io/kaitai_struct_compiler/552, the step

cat /dev/null | sbt test

exits with

[warn] No sbt.version set in project/build.properties, base directory: /home/circleci/repo
[info] Set current project to repo (in build file:/home/circleci/repo/)
[success] Total time: 3 s, completed May 11, 2020 10:08:51 AM

It doesn't seem to actually execute any tests.

On my local machine running sbt test results in

[info] Run completed in 2 seconds, 592 milliseconds.
[info] Total number of tests run: 1093
[info] Suites: completed 11, aborted 0
[info] Tests: succeeded 936, failed 157, canceled 0, ignored 0, pending 0
[info] *** 157 TESTS FAILED ***
[error] Failed tests:
[error]         io.kaitai.struct.exprlang.ExpressionsSpec
[error]         io.kaitai.struct.translators.TranslatorSpec
[error]         io.kaitai.struct.ErrorMessagesSpec
[error]         io.kaitai.struct.exprlang.Ast$Test
[error] (compilerJVM / Test / test) sbt.TestsFailedException: Tests unsuccessful
@generalmimon
Copy link
Member

generalmimon commented May 16, 2020

The compiler tests are run, but from a kaitai_struct Travis CI build instead: https://travis-ci.org/github/kaitai-io/kaitai_struct/builds/687910393#L1212

The script running it is called translator-tests.

The main CI build is set up on the umbrella repository in the Travis CI, which handles almost everything. It builds the compiler, then compiles the test KSY formats to source codes, and publishes the compiler to some package repositories if changed (see the CI infrastructure section in the docs). The second important build is on the ci_targets repo in Travis as well, this actually runs and tests the generated parsing source codes in the vast majority of languages. (C# testing takes place on AppVeyor, but that's basically it, AFAIK.)

There are also builds for generating format gallery, documentation and Web IDE, but they work on their own (apart from the devel Web IDE rebuild after the main build publishes a new compiler package to the npm repository).

I must admit that I didn't really know that we have some build running in Circle CI. Looking to it, I think the build set on compiler repo currently doesn't serve any purpose.

The internal compiler tests are currently sort of unused, they're not published anywhere in some readable form (unlike "integration" KSY test formats: https://ci.kaitai.io/). The old CI dashboard at http://kaitai.io/ci/ apparently displayed some of these results, but the new one at https://ci.kaitai.io/ still doesn't.

@vogelsgesang
Copy link
Contributor Author

Thanks for this detailed answer and all the pointers!
I read through the CI documentation before, but I was admittedly overwhelmed by it...
It seems I was side-tracked by an unused build on Circle-CI.

I am currently trying to wrap my head around KaitaiStruct, and I am having a hard time understanding how to validate my local setup and what the expectations around Pull Requests in this repository are...

In particular:

  • What is the "testing" strategy of KaitaiStruct? Are you trying to be "always green"?
  • Which type of local testing do you expect before proposing a Pull Request? Afaict, the CI-pipeline here in Github is mostly worthless to find issues before merging potentially broken Pull Requests because it will only kick off a build after 1) merging the PR into the sub-repository and 2) updating the referenced commit in the main repository. By the time the CI would indicate an issue with my changes, master would be already broken.
  • Which exact configuration of the main-project is the CI running? I am asking because, even before my PR Update JavaScript runtime #752, the CI was already green for the impacted test cases (see kaitai-io/ci_artifacts@99826cb), and I don't quite understand how the CI was able to find readBitsIntBe. It was very confusing that my local checkout of Kaitai was showing test failures which according to the CI shouldn't exist.
  • What to do about the compiler-tests. You mentioned they are currently unused. They seem to be disabled for 4 years by now (6fa1fb8). Does this mean they will be eventually removed and I don't have to bother updating them when making changes to the compiler? Or are you expecting PRs to update those test cases, too?

@generalmimon
Copy link
Member

generalmimon commented May 17, 2020

  • Which exact configuration of the main-project is the CI running? I am asking because, even before my PR Update JavaScript runtime #752, the CI was already green for the impacted test cases (see kaitai-io/ci_artifacts@99826cb), and I don't quite understand how the CI was able to find readBitsIntBe. It was very confusing that my local checkout of Kaitai was showing test failures which according to the CI shouldn't exist.

This is because there are prepare-* shell scripts for every language in the ci_targets repo, which are always run before any actual testing (see .travis.yml:184). Every prepare-* script runs git clone https://github.com/kaitai-io/kaitai_struct_*_runtime (e.g. prepare-javascript:5), which always gets the master commit of each runtime repo, so it's not relevant for the CI which commits of the runtime libraries are referenced from the kaitai_struct repo.

So the repos of the runtime libraries as submodules in the kaitai_struct repo are just for convenience when someone clones the repo, it doesn't affect CI builds.

Actually, in my opinion, the "main umbrella repo with all things as submodules" is a bad thing. It just increases overhead, because you have to constantly make sure that the submodules point to the master branches of each repo. When someone implements a new feature, it doesn't make sense to open a pull request on the main repo (e.g. #727), because it just references some commits of the submodules that are not connected to any branch (they should be on the top of master). That means that after the first person does some changes in the submodule repo, pushes some commits to the master branch and updates the submodule reference in the main repo, the changes introduced in the "main repo PR" merged earlier won't be incorporated and will be lost forever.

This of course confuses the newbies, just look at the people that forked the kaitai_struct repo. PRs don't make sense, so forking is useless as well.

Next thing is the PR response. Ideally, when someone opens a pull request in the compiler repo, it should trigger the CI build and provide the results in that repo, if it breaks/doesn't break something. This is not transparent currently, you don't know what the change does until you merge it and push a commit to the main repo referencing it (which triggers the build), unless you check it out locally.

@vogelsgesang
Copy link
Contributor Author

vogelsgesang commented May 19, 2020

Thanks for that long and detailed answer!

which always gets the master commit of each runtime repo, so it's not relevant for the CI which commits of the runtime libraries are referenced from the kaitai_struct repo

afaict, the CI build is only kicked off as soon as the master repository is updated, but will then pick the latest dependencies, independent of the referenced commits in the repo, right? This is, at least to me, an unexpected configuration...

Actually, in my opinion, the "main umbrella repo with all things as submodules" is a bad thing

I tend to agree. At least to me as a newbie, it would have been much easier to setup my local dev-environment if Kaitai would be a monorepo. Also, the benefits from a "multi-repository"-setup are not yet clear to me.
But then again: I have very little insight into Kaitai, so I am not really in a position to give any advice/propose any changes here...

Anyhow, I am not sure if there are any follow-up items out of this discussion.
Maybe remove the unused Circle-CI integration from the compiler repository? Not sure...
Feel free to just close this issue

@generalmimon
Copy link
Member

Feel free to just close this issue

OK, closing.

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

No branches or pull requests

2 participants