Fix #346, Adds library build, functional, and coverage test to CI by skliper · Pull Request #403 · nasa/osal

skliper · 2020-04-01T22:37:29Z

Describe the contribution
Fix #346, Adds library build, functional, and coverage test to CI

Testing performed
Steps taken to test the contribution:

CI from fork - https://travis-ci.com/github/skliper/osal/builds/157641624

Expected behavior changes
Just adds tests to CI

System(s) tested on

Hardware: AMD
OS: Ubuntu 18.04
Versions: bundle + warning fix from previous pull request + this change

Additional context
None

Third party code
None

Contributor Info - All information REQUIRED for consideration of pull request
Jacob Hageman - NASA/GSFC

skliper · 2020-04-09T20:40:15Z

@astrogeco we seem to be skipping a few pull requests that got submitted but aren't marked ready for CCB. Can we just assume if they are new they should be discussed (as long as they aren't tagged w/ WIP)?

skliper · 2020-04-09T20:41:09Z

Or review everything that isn't marked NOT ready?

astrogeco · 2020-04-13T17:54:09Z

Or review everything that isn't marked NOT ready?

Good point, though we've been keeping the agenda pretty full despite the fact we're missing those. Ill take a look.

skliper · 2020-04-15T20:55:23Z

Rebased and updated per #404...

skliper · 2020-04-15T21:08:07Z

Now cppcheck is --quiet to clean up the CI log.

stanislaw · 2020-04-16T11:15:54Z

It is great to see that this is being prepared. I have been waiting for it since I opened the macOS support branch.

I have a few comments on what is being implemented here. They are in no way intended to be blocking the current changeset which is a great enhancement.

From quite some experience of maintaining private and public CI setups, I would highly recommend you to not put your CI commands directly into the .travis.yml. The reason is that you cannot run them run them locally on your developer machine easily to reproduce 100% what is run on CI. The sequence of commands being introduced is already quite advanced and without being put into some executable scripts you can get into a very inconvenient situation when it will be easier run the pushes/PRs on CI instead of testing them with convenience locally.

I have seen many options but the following three are on my active list:

a) Good old Makefile.

Extremely typical example I have been working with recently: doorstop/Makefile.

All commands can be put into the tasks such as: build, test, gcov, etc. These can be called from .travis.yml and from a local developer machine.

The experience taught me and my colleagues that this approach does not scale well when you get a large number of tasks and when you have to parametrize your script in advanced way.

b) Ansible playbooks

I am sure you are aware of Ansible playbooks.

This is what @AlexDenisov and I are using in our Mull project with a great convenience as we have to scale our tool across many versions of OSes and LLVM. Please see examples: https://github.com/mull-project/mull/blob/master/infrastructure/macos-playbook.yaml.

c) Invoke

This is a Python tool which gives you a combination of Makefile tasks functionality and Python scripting. See the example here: https://github.com/stanislaw/FileCheck.py/blob/master/tasks.py. What is run by CI can run 1-1 locally on all supported platforms: Linux, macOS and Windows.

These days, If I had to start a new project equivalent in scale, importance and complexity to such of the nasa/osal, I would go for a combination of the approaches b) and c):

Keeping the infrastructure/dependencies logic in Ansible playbooks (the Mull example above) while having all tasks such as _functional_test in the modularized suite of Python files based on Invoke (the FileCheck example above).

I hope this advice could help you in one or another way.

I have some more comments about the changeset and I will put them in the form of specific comments.

stanislaw · 2020-04-16T11:19:57Z

src/bsp/pc-linux/build_options.cmake

+# Note - although GCC understands the same flags for compile and link here, this may
+# not be true on all platforms so the compile and link flags are specified separately.
+if (NOT CMAKE_CROSSCOMPILING)
+  set(UT_COVERAGE_COMPILE_FLAGS -pg --coverage)


This seems to be non-portable:

clang: error: the clang compiler does not support -pg option on versions of OS X 10.9 and later

Another note, even if it might be beyond the scope of this changeset. Usually such functionality is created in separate files like cmake/coverage.cmake to not mix the logic of the targets with the logic of the coverage.

The same applies to things like clang-format, clang-tidy, other code linters, spell checking, etc.

One good example can be found here: https://github.com/StableCoder/cmake-scripts.

skliper · 2020-04-16T13:13:12Z

@stanislaw Great suggestions, thanks! Definitely a goal to improve.

skliper · 2020-04-20T21:34:06Z

Rebased on master

astrogeco · 2020-04-22T16:56:13Z

CCB 20200422 - APPROVED

NOTE - Comments/documentation only, no logic updates Also fixes nasa#403, resolves all warnings Also fixes nasa#536, comment typo in CFE_SB_RcvMsg implementation header

skliper added the enhancement label Apr 1, 2020

skliper added this to the 5.1.0 milestone Apr 1, 2020

skliper mentioned this pull request Apr 2, 2020

Update OSAL Configuration Guide, build instructions, directory references, etc #384

Closed

skliper added the CCB:Ignore Incomplete Pull Request with open actions. label Apr 9, 2020

skliper force-pushed the fix346-add-ci-test branch 2 times, most recently from d97afc0 to 2373364 Compare April 15, 2020 20:50

skliper added CCB:Ready Pull request is ready for discussion at the Configuration Control Board (CCB) and removed CCB:Ignore Incomplete Pull Request with open actions. labels Apr 15, 2020

skliper force-pushed the fix346-add-ci-test branch from 2373364 to 6967079 Compare April 15, 2020 20:58

skliper mentioned this pull request Apr 15, 2020

"duplicate symbol '_main'" on macOS when building tests #363

Closed

skliper force-pushed the fix346-add-ci-test branch from 6967079 to af96893 Compare April 15, 2020 21:07

skliper force-pushed the fix346-add-ci-test branch from af96893 to 221fea2 Compare April 15, 2020 21:13

stanislaw reviewed Apr 16, 2020

View reviewed changes

This was referenced Apr 16, 2020

CI improvements - better support of local and cross-platform testing #419

Closed

Separate cmake coverage logic and resolve clang support issue #420

Open

Fix nasa#346, Add test and coverage report to CI

6e1f096

skliper force-pushed the fix346-add-ci-test branch from 221fea2 to 6e1f096 Compare April 20, 2020 21:32

astrogeco added the continuous-integration label Apr 21, 2020

stanislaw mentioned this pull request Apr 22, 2020

WIP: os/posix: port of the posix implementation to macOS #352

Closed

14 tasks

skliper added CCB - 20200422 and removed CCB:Ready Pull request is ready for discussion at the Configuration Control Board (CCB) labels Apr 22, 2020

skliper added the CCB:Approved Indicates code review and approval by community CCB label Apr 22, 2020

astrogeco added the IC-20200422 label Apr 27, 2020

astrogeco changed the base branch from master to integration-candidate April 27, 2020 23:00

astrogeco merged commit a138fba into nasa:integration-candidate Apr 28, 2020

astrogeco mentioned this pull request Apr 30, 2020

Integration Candidate: 2020-04-22 #440

Merged

astrogeco removed enhancement continuous-integration labels Sep 22, 2020

skliper deleted the fix346-add-ci-test branch February 1, 2021 21:36

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix #346, Adds library build, functional, and coverage test to CI#403

Fix #346, Adds library build, functional, and coverage test to CI#403
astrogeco merged 1 commit intonasa:integration-candidatefrom
skliper:fix346-add-ci-test

skliper commented Apr 1, 2020

Uh oh!

skliper commented Apr 9, 2020

Uh oh!

skliper commented Apr 9, 2020

Uh oh!

astrogeco commented Apr 13, 2020

Uh oh!

skliper commented Apr 15, 2020

Uh oh!

skliper commented Apr 15, 2020

Uh oh!

stanislaw commented Apr 16, 2020 •

edited

Loading

Uh oh!

stanislaw Apr 16, 2020

Uh oh!

stanislaw Apr 16, 2020

Uh oh!

skliper Apr 16, 2020

Uh oh!

skliper commented Apr 16, 2020

Uh oh!

skliper commented Apr 20, 2020

Uh oh!

astrogeco commented Apr 22, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

skliper commented Apr 1, 2020

Uh oh!

skliper commented Apr 9, 2020

Uh oh!

skliper commented Apr 9, 2020

Uh oh!

astrogeco commented Apr 13, 2020

Uh oh!

skliper commented Apr 15, 2020

Uh oh!

skliper commented Apr 15, 2020

Uh oh!

stanislaw commented Apr 16, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

stanislaw Apr 16, 2020

Choose a reason for hiding this comment

Uh oh!

stanislaw Apr 16, 2020

Choose a reason for hiding this comment

Uh oh!

skliper Apr 16, 2020

Choose a reason for hiding this comment

Uh oh!

skliper commented Apr 16, 2020

Uh oh!

skliper commented Apr 20, 2020

Uh oh!

astrogeco commented Apr 22, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

stanislaw commented Apr 16, 2020 •

edited

Loading