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

Tiny change since the tests are more than unit tests #765

Merged
merged 1 commit into from
Nov 1, 2022

Conversation

fzyzcjy
Copy link
Contributor

@fzyzcjy fzyzcjy commented Nov 1, 2022

(This is a super tiny change that I realized when reading the README)

IMHO, unit tests, compared with integration tests or end-to-end tests or other tests, often means something that only tests a single method/function/class/etc, and often has a lot of stubs and mocks so it is far from a typical/real usage scenario. On the other hand, integration tests or e2e tests mock less and are more like the real case.

Tianshou says:

... tests include the full agent training procedure for all of the implemented algorithms

It seems that this is more than unit test, and falls into the category of integration or even e2e tests.

  • I have marked all applicable categories:
    • exception-raising fix
    • algorithm implementation fix
    • documentation modification
    • new feature
  • I have reformatted the code using make format (required)
  • I have checked the code using make commit-checks (required)
  • If applicable, I have mentioned the relevant/related issue(s)
  • If applicable, I have listed every items in this Pull Request below

@codecov-commenter
Copy link

codecov-commenter commented Nov 1, 2022

Codecov Report

Merging #765 (73b94ca) into master (d42a5fb) will decrease coverage by 0.01%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master     #765      +/-   ##
==========================================
- Coverage   91.24%   91.22%   -0.02%     
==========================================
  Files          71       71              
  Lines        5082     5082              
==========================================
- Hits         4637     4636       -1     
- Misses        445      446       +1     
Flag Coverage Δ
unittests 91.22% <ø> (-0.02%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
tianshou/data/buffer/her.py 90.58% <0.00%> (-1.18%) ⬇️

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@Trinkle23897 Trinkle23897 merged commit 7ff12b9 into thu-ml:master Nov 1, 2022
BFAnas pushed a commit to BFAnas/tianshou that referenced this pull request May 5, 2024
IMHO, unit tests, compared with integration tests or end-to-end tests or
other tests, often means something that only tests a single
method/function/class/etc, and often has a lot of stubs and mocks so it
is far from a typical/real usage scenario. On the other hand,
integration tests or e2e tests mock less and are more like the real
case.

Tianshou says:

> ... tests include the full agent training procedure for all of the
implemented algorithms

It seems that this is more than unit test, and falls into the category
of integration or even e2e tests.
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