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.

Sign up for GitHub

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

Jump to bottom

Add new test crate #368

Merged
merged 2 commits into from
Dec 15, 2022
Merged

Add new test crate #368

merged 2 commits into from
Dec 15, 2022

Conversation

haerdib
Copy link
Contributor

@haerdib haerdib commented Dec 9, 2022 •
edited
Loading

Test the full functionality of the api client.

Update CI build:

Not CI related:

  • Remove example get_existential_deposit. That's not really an useful example and is covered by the tests now.

closes #353

@haerdib haerdib self-assigned this Dec 9, 2022
@haerdib haerdib changed the title Add test crate Add test crate and clean up CI builds Dec 12, 2022
@haerdib haerdib marked this pull request as ready for review December 14, 2022 06:47
@haerdib haerdib changed the title Add test crate and clean up CI builds Add test crate, clean up CI builds and Bug fixes Dec 14, 2022
@haerdib haerdib requested a review from clangenb December 14, 2022 06:49
@haerdib haerdib changed the title Add test crate, clean up CI builds and Bug fixes Bug fixed and new test crate Dec 14, 2022
@haerdib haerdib changed the title Bug fixed and new test crate Bug fixes and new test crate Dec 14, 2022
@haerdib haerdib added the F2-bug Something isn't working label Dec 14, 2022
@haerdib haerdib mentioned this pull request Dec 14, 2022
Closed
1 task
clangenb
clangenb reviewed Dec 14, 2022
View reviewed changes
Copy link
Collaborator

@clangenb clangenb left a comment

Choose a reason for hiding this comment

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

Nice cleanup thanks! However, I have some questions. Also, I believe that this PR solves different issues at the same time. Is there a reason that you can't separate this PR?

testing/examples/author_tests.rs
@@ -0,0 +1,98 @@
/*
Copy link
Collaborator

Choose a reason for hiding this comment

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

Are these now tests or examples :D? I suggest that we stay consistent with the naming. We can go either for tests or examples, but I am against this schizophrenic scheme.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Actually i think we should either make it bins, then we don't need to add the --example flag all the time, what do you think?

Copy link
Contributor Author

@haerdib haerdib Dec 14, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

The good thing about examples is that they are built automatically with cargo test (but not run).

Binaries are also possible. Implication: Binaries are built with normal "cargo build --release". Not sure if that's a drawback or not.
What's certainly a drawback: CI needs to be adapted quite a bit, because the binaries are simply stored in the release folder. Or is there a way to define an output folder?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Nope, the change would imply that we need to copy it from another folder. Hmm, it is hard to say.

Ok, I will back off. If you think this is the suitable solution, I happily go with it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Suitable... not sure. But the simplest one at leat 😄

@haerdib haerdib mentioned this pull request Dec 15, 2022
Merged
@haerdib haerdib changed the title Bug fixes and new test crate Add new test crate Dec 15, 2022
@haerdib haerdib marked this pull request as draft December 15, 2022 09:17
@haerdib haerdib requested a review from echevrier December 15, 2022 10:49
@haerdib haerdib marked this pull request as ready for review December 15, 2022 10:51
@haerdib haerdib removed the F2-bug Something isn't working label Dec 15, 2022
@haerdib
add tests
42828d3 
fix tests

fix features in toml

add tests to example

fmt

clean up ci.yml

fix fmt

rename fmt to taplo fmt

add job name

 add some nice naming

renove extra nem

fix CI

add --release and add contains

fix naming

use release instead of (hugeee) debug

fix contains

fix generic_storage_tests

fix payment info queries

fix finalized

update tests

fix subscription

fix compilation

fix rebase changes

fix tests

fix tests

add block hash

readd seperate featrues to ci

fix comments

add OldWeight to runtime dispatch info

rename bin folder to examples
@haerdib
remove ed example
e3cfe31
echevrier
echevrier approved these changes Dec 15, 2022
View reviewed changes
Copy link
Contributor

@echevrier echevrier left a comment

Choose a reason for hiding this comment

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

LGTM

@haerdib haerdib merged commit 742009e into master Dec 15, 2022
@haerdib haerdib deleted the bh/add-test-crate branch December 15, 2022 16:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@clangenb clangenb clangenb approved these changes

@echevrier echevrier echevrier approved these changes

Assignees

@haerdib haerdib

Labels
E1-breaksnothing F3-test fix tests or CI
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Introduce a test suite for system testing
3 participants
@haerdib @clangenb @echevrier