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

Migrated to doctest #2436

Merged
merged 1 commit into from
Apr 4, 2023
Merged

Migrated to doctest #2436

merged 1 commit into from
Apr 4, 2023

Conversation

JohanMabille
Copy link
Member

No description provided.

@JohanMabille JohanMabille force-pushed the doctest branch 8 times, most recently from 6282f36 to 02abcac Compare April 3, 2023 15:55
Copy link
Member

@Klaim Klaim left a comment

Choose a reason for hiding this comment

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

LGTM

@JohanMabille JohanMabille force-pushed the doctest branch 3 times, most recently from bd2eab8 to 1088f0d Compare April 4, 2023 07:44
Copy link
Member

@AntoinePrv AntoinePrv left a comment

Choose a reason for hiding this comment

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

Thanks for doing this! I'm looking forward to nicer testing!
I mostly checked the test I wrote and everything looks good.

#include "mamba/core/activation.hpp"

#include "doctest/doctest.h"
Copy link
Member

Choose a reason for hiding this comment

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

Nit:

Suggested change
#include "doctest/doctest.h"
#include <doctest/doctest.h>

|| std::holds_alternative<ProblemsGraph::ConstraintNode>(g.node(n))
);
}
std::vector<decltype(&create_basic_conflict)> pb_values = {
Copy link
Member

Choose a reason for hiding this comment

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

Nit: std::array

}

TEST_P(Problem, simplify_conflicts)
TEST_SUITE("satifiability_error")
Copy link
Member

Choose a reason for hiding this comment

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

Adding a mental note to come back on this test to share the resources without static. We have ~10 static Pools here so 10x repodata.json

@JohanMabille JohanMabille force-pushed the doctest branch 3 times, most recently from b750481 to 720e724 Compare April 4, 2023 14:21
@JohanMabille JohanMabille merged commit ea834b5 into mamba-org:main Apr 4, 2023
@JohanMabille JohanMabille deleted the doctest branch April 4, 2023 15:43
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.

3 participants