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

Fix 834 and Add Unit Tests (dependent on #780) #849

Open
wants to merge 42 commits into
base: master
Choose a base branch
from
Open

Fix 834 and Add Unit Tests (dependent on #780) #849

wants to merge 42 commits into from

Conversation

mrpilot2
Copy link
Contributor

@mrpilot2 mrpilot2 commented Dec 7, 2015

I've created a unit test that reproduces the segfault, no matter if filte1.4, or flite2.0 is installed on my Ubuntu 14.04

The unit test works fine for flite1.4 and flite2.0, if commit 5f0890a is reverted

create a static library of all apmplanner classes except of main, moved qgroundcontrol.pro to apmplanner_core.pro to keep the build process the same as before
removed old project configuration and source files from qgcunittest.pro because that is now part of the new common apmplanner-core library

added a test for the qt test framwork to make sure the new setup works as expected
versions, copied from ApmFirmwareConfig.cc

added unit tests for version comparator
in ApmFirmwareConfig.cc

This bug was already fixed in AutoUpdateCheck.cc
commit 8de0f15
… defined in the regex,

therefore it is always 2 and the if statement is useless
…rsion string in one place

fixed uninitialized variable warning
…s per group, allow muliple digits in build number
Conflicts:
	qgroundcontrol.pro
whenever a waypoint is dragged, go through the list of waypoints and find all other selected waypoints - shift these waypoints by the same lat/long difference as the initial waypoint was moved
… SpinBox in one list to fix duplicate for loop
… (issue #40)

Verbose flag is always true and debug flag always false - therefore removed these flags, if another verbosity level is needed the logging system should be used
…:Type and comparison between enum QVariant::Type and enum QMetaType::Type

according to Qt documentation QVariant::type() should be interpreted as QMetaType::Type
@billbonney
Copy link
Member

Thanks for finding a solution. I can't merge this commit until the fix is against current master. I know I need to pull in the other two PRs you have, but this PR is essentially everything is an 'omnibus' PR.

I can cherry-pick dc60229, but it much easier to merger PRs against the current master without the 'noise' of other PRs

@mrpilot2
Copy link
Contributor Author

mrpilot2 commented Dec 8, 2015

Sorry for that, but I wanted to cover my changes with a unit test, that's why I used my master branch.

@billbonney
Copy link
Member

No worries. I'll look at the unit test you added again, the problem was I had some OSX build issue, and I hadn't had time to resolve (I'll try and find some time to fix that as it's a good PR)

@billbonney
Copy link
Member

cherry-picked fix with 6f4ec1f

That still leaves the unit test case pr and then the two test cases

@billbonney billbonney changed the title Fix 834 Fix 834 and Add Unit Tests (dependent on #780) Dec 9, 2015
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.

2 participants