Skip to content

Add unit tests for Branch module #1445

Merged
merged 15 commits into from
Dec 6, 2020
Merged

Add unit tests for Branch module #1445

merged 15 commits into from
Dec 6, 2020

Conversation

graudtv
Copy link
Contributor

@graudtv graudtv commented Dec 4, 2020

Correspond to issue #512

@codecov
Copy link

codecov bot commented Dec 4, 2020

Codecov Report

Merging #1445 (8505cbb) into master (a39e74d) will increase coverage by 0.00%.
The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff            @@
##           master    #1445    +/-   ##
========================================
  Coverage   99.91%   99.91%            
========================================
  Files         135      137     +2     
  Lines       11014    11128   +114     
========================================
+ Hits        11005    11119   +114     
  Misses          9        9            
Impacted Files Coverage Δ
simulator/modules/branch/branch.cpp 100.00% <ø> (ø)
simulator/modules/branch/branch.h 100.00% <ø> (ø)
simulator/modules/branch/t/unit_test.cpp 100.00% <100.00%> (ø)
simulator/modules/t/test_instr.h 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a39e74d...8505cbb. Read the comment docs.

Copy link
Member

@pavelkryukov pavelkryukov left a comment

Choose a reason for hiding this comment

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

Please take into account my comments and CodeCov note.

simulator/modules/branch/t/unit_test.cpp Outdated Show resolved Hide resolved
simulator/modules/branch/t/unit_test.cpp Outdated Show resolved Hide resolved
@graudtv
Copy link
Contributor Author

graudtv commented Dec 6, 2020

Codecov makes a mistake. That line is inlined and optimized, so there is no call, but it is necessary

@pavelkryukov
Copy link
Member

Codecov makes a mistake. That line is inlined and optimized, so there is no call, but it is necessary

Then add a call in a new test case.
Now, we can write here anything (1 / 0) and tests won't cover it.

@graudtv
Copy link
Contributor Author

graudtv commented Dec 6, 2020

It will also be optimized. Calling something like cout << taken_and_predicted_branch; does nothing;

@pavelkryukov
Copy link
Member

No, it is not. For code coverage, we disable all optimizations. See other code lines in headers — they are not inlined.

@pavelkryukov
Copy link
Member

You may add some dummy output (return out << “BranchTestInstr”)

pavelkryukov
pavelkryukov previously approved these changes Dec 6, 2020
@pavelkryukov
Copy link
Member

I've sent a patch with test for BranchTestInstr::operator<<.

Test BranchTestInstr output
@pavelkryukov pavelkryukov merged commit d2ed627 into MIPT-ILab:master Dec 6, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants