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 broken unit tests #680

Merged
merged 17 commits into from
Oct 1, 2018
Merged

Fix broken unit tests #680

merged 17 commits into from
Oct 1, 2018

Conversation

schillic
Copy link
Member

@schillic schillic commented Sep 30, 2018

EDIT: Does not close the issue #71.

@schillic
Copy link
Member Author

I vote for activating the other unit tests in the builds. This will take a bit longer, but we will notice when problems are introduced.
Polyhedra tests are only available in v0.6 at the moment (that package is also needed for plotting), so I propose the following settings.

# old, unchanged
global test_suite_basic = true
global test_suite_doctests = VERSION >= v"0.7-" # only run doctests with new Julia version
# new
global test_suite_polyhedra = VERSION < v"0.7-" # only run doctests with old Julia version (for now)
global test_suite_plotting = VERSION < v"0.7-" # only run doctests with old Julia version (for now)

@schillic schillic requested a review from mforets September 30, 2018 20:32
@mforets
Copy link
Member

mforets commented Sep 30, 2018

I vote for activating the other unit tests in the builds. This will take a bit longer, but we will notice when problems are introduced.

I agree too. Once Polyhedra tests pass in 0.7 too we can switch all default tests to true.

@schillic
Copy link
Member Author

I had to undo the commit for strict doctests because Optim has many undocumented functions and for some reason Documenter complains about them (although we explicitly say
modules = Module[LazySets, Approximations]).

@mforets
Copy link
Member

mforets commented Sep 30, 2018

Does strict=true imply that if we have an undocumented function the build breaks? (If yes, i think that's "too strict".)

@schillic
Copy link
Member Author

Does strict=true imply that if we have an undocumented function the build breaks? (If yes, i think that's "too strict".)

Unfortunately, that is the case (and there is no fine-tuning option ).

@mforets
Copy link
Member

mforets commented Sep 30, 2018

Now or later, the additional packages used for tests can be added in the REQUIRE file
https://github.com/JuliaReach/LazySets.jl/blob/master/test/REQUIRE

@mforets
Copy link
Member

mforets commented Sep 30, 2018

Unfortunately, that is the case (and there is no fine-tuning option ).

Alright. Documented here, i'd like strict=true but it doesn't break if checkdocs fails. But it seems like if strict=true then checkdocs option is ignored?

@mforets
Copy link
Member

mforets commented Oct 1, 2018

i suspect (but i'm not 100% sure) that the failure with the v1.0 is the new restriction of the package manager to refuse installing packages which are known not to work in > 0.7. the error message says

 Resolving package versions...
ERROR: Unsatisfiable requirements detected for package CDDLib [3391f64e]:
 CDDLib [3391f64e] log:
 ├─possible versions are: [0.0.1, 0.1.0-0.1.4, 0.2.0-0.2.3, 0.3.0-0.3.2] or uninstalled
 ├─restricted to versions 0.3.2-* by an explicit requirement, leaving only versions 0.3.2
 └─restricted by julia compatibility requirements to versions: uninstalled — no versions left

@mforets
Copy link
Member

mforets commented Oct 1, 2018

finally

screen shot 2018-09-30 at 23 08 06

let's temporarily allow failure in the 1.0 build until the issue with the dependency is fixed.

@schillic
Copy link
Member Author

schillic commented Oct 1, 2018

I would rather remove CDDLib/Polyhedra from the test/REQUIRE file because

  1. we do not use these packages in the newer versions, and
  2. for the v0.6 build we need to write the Pkg.add commands explicitly.

@mforets mforets merged commit fd872f3 into master Oct 1, 2018
@schillic schillic deleted the schillic/tests branch October 1, 2018 11:42
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