ENT-3999: Atexit code coverage #3197

craigcomstock · 2018-08-01T20:15:50Z

This PR is focused on only increasing code coverage to core as-is for exit() calls. This is in preparation for the refactor away from atexit() for Windows' sake.

Will squash once I get as much coverage as is reasonable.

codecov · 2018-08-03T18:26:22Z

Codecov Report

Merging #3197 into master will increase coverage by 1.27%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master    #3197      +/-   ##
==========================================
+ Coverage   61.87%   63.14%   +1.27%     
==========================================
  Files         211      212       +1     
  Lines       46332    46360      +28     
==========================================
+ Hits        28669    29276     +607     
+ Misses      17663    17084     -579

Impacted Files	Coverage Δ
cf-serverd/server_transform.c	`26.08% <ø> (+26.08%)`	⬆️
libenv/unix_iface.c	`85.49% <100%> (+0.89%)`	⬆️
libpromises/files_names.c	`72.34% <0%> (-0.71%)`	⬇️
libpromises/locks.c	`81.66% <0%> (-0.25%)`	⬇️
cf-serverd/cf-serverd.c	`66.66% <0%> (ø)`
libpromises/eval_context.c	`86.89% <0%> (+0.15%)`	⬆️
libpromises/loading.c	`84.52% <0%> (+0.37%)`	⬆️
libpromises/policy.c	`71.46% <0%> (+0.88%)`	⬆️
libutils/string_lib.c	`65.97% <0%> (+1.87%)`	⬆️
... and 16 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 11b10fb...c23c55f. Read the comment docs.

craigcomstock · 2018-08-08T21:39:49Z

craigcomstock · 2018-08-08T21:43:59Z

cf-serverd/server_transform.c

for what it's worth... I tried some

#pragma GCC diagnostic push #pragma GCC diagnostic ignored "-Wunused-label" ... #pragma GCC diagnostic pop

But it didn't seem to remove the warnings when I was compiling locally. :(

as it turns out these unused labels don't seem to break travis/jenkins even though -Werror seems to be enabled. I fixed cf-monitor/env_monitor.c label with a gcc Label Attribute like:

__attribute__((unused));

So maybe just to be safe I will add that to these labels as well.

craigcomstock · 2018-08-21T16:17:49Z

craigcomstock · 2018-08-22T03:14:29Z

vpodzime

I think the line "hard-coded" numbers are an issue. Maybe we should use something like grep -n to get the line numbers of particular labels and then use these "adapting" line numbers in the gdb scripts?

vpodzime · 2018-08-22T11:16:44Z

.travis.yml

Also add -ggdb3.

Yes, I worked on a grep -n solution for a little while but then dropped it. Will pick it back up now and try to get it done. :)

@vpodzime do you think -g3 (-ggdb3 equivalent I think) would fix the labels problem I had? Maybe academic now since I likely have the grep -n solution working.

vpodzime · 2018-08-22T11:20:11Z

tests/acceptance/16_cf-serverd/add_access_constraints_fail.gdb

Whose gonna update these line numbers when the file(s) is/are changed?

vpodzime · 2018-08-22T11:20:52Z

travis-scripts/script.sh

What is this about?

That was an attempt by me to streamline the test cycle with jenkins and such... by settings a TESTS var and only running the test I wanted to check on. It's half-baked so will remove it. Thanks for the catch.

craigcomstock · 2018-08-24T04:39:41Z

tests/acceptance/16_cf-serverd/keep_promises_fail.cf

@vpodzime something like the above? It seemed to work pretty well. I will work on the other two instances when I can.

Yeah, something like that.

craigcomstock · 2018-08-24T04:40:20Z

cf-serverd/server_transform.c

@vpodzime how about this? I like it pretty well. Need to implement the other two instances and test on travis/jenkins

I think using a more noticeable prefix like GDB: would be better. Also it would be nice if the comment was aligned with the code. But it's not a big deal.

vpodzime · 2018-08-24T09:16:19Z

tests/acceptance/16_cf-serverd/keep_promises_fail.cf

This would be better done with a files: promise, right?

craigcomstock · 2018-08-27T21:21:41Z

@cf-bottom jenkins build please

cf-bottom · 2018-08-27T21:24:25Z

Alright, I triggered a build:

https://ci.cfengine.com/job/pr-pipeline/1055/

craigcomstock · 2018-08-30T21:45:54Z

@cf-bottom jenkins builds please

cf-bottom · 2018-08-30T21:49:50Z

Alright, I triggered a build:

https://ci.cfengine.com/job/pr-pipeline/1080/

Add tests to increase code coverage around exit()/atexit() calls.

… still work

…k with that

…file:linenumber syntax for gdb driven tests

…st policy filename

craigcomstock · 2018-09-07T19:38:23Z

made this a WIP in hopes that a combined refactor+tests PR works better: #3252

craigcomstock · 2018-10-15T21:40:49Z

abandoning due to complexity of testing exit() calls.

craigcomstock added the WIP Work in Progress label Aug 1, 2018

craigcomstock force-pushed the atexit-code-coverage branch from d14a79d to 867ebb6 Compare August 1, 2018 20:16

craigcomstock force-pushed the atexit-code-coverage branch 2 times, most recently from e8a8f95 to e5f2cb8 Compare August 8, 2018 14:05

craigcomstock commented Aug 8, 2018

View reviewed changes

craigcomstock mentioned this pull request Aug 20, 2018

ENT-3756: Refactor exit to exitaftercleanup #3179

Closed

craigcomstock force-pushed the atexit-code-coverage branch from e3c802f to 9ebf3e3 Compare August 21, 2018 15:44

craigcomstock removed the WIP Work in Progress label Aug 22, 2018

vpodzime reviewed Aug 22, 2018

View reviewed changes

craigcomstock commented Aug 24, 2018

View reviewed changes

vpodzime reviewed Aug 24, 2018

View reviewed changes

tests/acceptance/16_cf-serverd/keep_promises_fail.cf Outdated

Copy link

Contributor

vpodzime Aug 24, 2018

Choose a reason for hiding this comment

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

This would be better done with a files: promise, right?

craigcomstock force-pushed the atexit-code-coverage branch from 37d4ca0 to 9e0fef4 Compare August 30, 2018 21:27

craigcomstock added 10 commits September 4, 2018 14:48

ENT-3756: cf-agent intermittently hangs on windows

1ceb7d2

Add tests to increase code coverage around exit()/atexit() calls.

cleanup, but no squash yet, let travis/jenkins test and see if things…

1e432f4

… still work

using gdb without explicit path seems to work fine in general so stic…

2d9a35d

…k with that

try to set gdb breakpoint dynamically from C code comment

6544bc8

sqaushme

ad2d2d8

fixup gdb templating to use files promise and grep -n for line numbers

313fb78

sq

9478df6

experiment with removing gcc-7 and -O0 since we are now using simple …

2b1e9de

…file:linenumber syntax for gdb driven tests

experiment with only changing CFLAGS and not using gcc-7

87a5678

junk commit to kick github

add45e5

craigcomstock added 3 commits September 4, 2018 14:48

debug gdb source directories on jenkins buildslaves, squashme

2413331

add GDB: prefix to make sure we dont catch other references to the te…

0e88ab4

…st policy filename

test gdb tests on redhat_6 buildslave

ea8474c

craigcomstock force-pushed the atexit-code-coverage branch from f4c325b to ea8474c Compare September 4, 2018 19:49

debug redhat_6

c23c55f

craigcomstock added the WIP Work in Progress label Sep 7, 2018

craigcomstock closed this Oct 15, 2018

ENT-3999: Atexit code coverage #3197

ENT-3999: Atexit code coverage #3197

Uh oh!

Conversation

craigcomstock commented Aug 1, 2018

Uh oh!

codecov bot commented Aug 3, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

craigcomstock commented Aug 8, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

craigcomstock Aug 8, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

craigcomstock commented Aug 21, 2018

Uh oh!

craigcomstock commented Aug 22, 2018

Uh oh!

vpodzime left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

craigcomstock commented Aug 27, 2018

Uh oh!

cf-bottom commented Aug 27, 2018

Uh oh!

craigcomstock commented Aug 30, 2018

Uh oh!

cf-bottom commented Aug 30, 2018

Uh oh!

craigcomstock commented Sep 7, 2018

Uh oh!

craigcomstock commented Oct 15, 2018

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

3 participants

codecov bot commented Aug 3, 2018 •

edited

Loading

craigcomstock commented Aug 8, 2018 •

edited

Loading

craigcomstock Aug 8, 2018 •

edited

Loading