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

[FEATURE] Create unit test for rust code #1615

Merged
merged 26 commits into from
Aug 11, 2024

Conversation

IshanGrover2004
Copy link
Contributor

@IshanGrover2004 IshanGrover2004 commented Jun 1, 2024

In raising this pull request, I confirm the following (please check boxes):

  • I have read and understood the contributors guide.
  • I have checked that another pull request for this purpose does not exist.
  • I have considered, and confirmed that this submission will be valuable to others.
  • I accept that this submission may not be used, and the pull request closed at the will of the maintainer.
  • I give this submission freely, and claim no ownership to its content.
  • I have mentioned this change in the changelog.

My familiarity with the project is as follows (check one):

  • I have never used CCExtractor.
  • I have used CCExtractor just a couple of times.
  • I absolutely love CCExtractor, but have not contributed previously.
  • I am an active contributor to CCExtractor.

Creating Unit test cases for rust modules(decoder + hardsubx).
Closes #911

@IshanGrover2004
Copy link
Contributor Author

The decoder moulde's unit test done @PunitLodha

@IshanGrover2004 IshanGrover2004 changed the title [WIP] [FEATURE] Create unit test for rust code [FEATURE] Create unit test for rust code Jun 14, 2024
@PunitLodha PunitLodha self-requested a review June 14, 2024 12:50
@PunitLodha
Copy link
Member

@IshanGrover2004 Can you also add a github CI check for running the unit tests

@IshanGrover2004
Copy link
Contributor Author

Sure @PunitLodha

@ccextractor-bot

This comment was marked as outdated.

@IshanGrover2004 IshanGrover2004 force-pushed the create-unit-testing branch 2 times, most recently from 2b2eb7a to 17355f2 Compare July 17, 2024 08:15
@IshanGrover2004
Copy link
Contributor Author

IshanGrover2004 commented Jul 18, 2024

@PunitLodha @cfsmp3 You can proceed for merging. I have added CI + all modules unit test cases

.github/workflows/format.yml Outdated Show resolved Hide resolved
src/rust/src/decoder/window.rs Outdated Show resolved Hide resolved
decoder.is_header_parsed = true;
decoder.is_active = true;
decoder.report_enabled = true;
decoder.packet = vec![0xC2, 0x23, 0x45, 0x67, 0x00, 0x00];
Copy link
Member

Choose a reason for hiding this comment

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

If you find some time, you can add comments here explaining what exactly the packet is here, the header the data

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not possible tbh, bcs these are based on random values resulting of manipulation done in the functions & parameters passed as inputs. On based on that the resulted values are tested, I did not planned for any particular data.

Comment on lines +267 to +269
assert_eq!(decoder.packet[0], 0x01);
assert_eq!(decoder.packet[1], 0x02);
assert_eq!(decoder.packet_length, 2);
Copy link
Member

Choose a reason for hiding this comment

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

Also, you could explain why we do these asserts. For example, why should decoder.packet[0] be equal to 0x01

Comment on lines +1262 to +1266
decoder.windows[0].attribs.print_direction =
dtvcc_window_pd::DTVCC_WINDOW_PD_RIGHT_LEFT as i32;
decoder.process_cr(&mut encoder, &mut timing, no_rollup);
assert_eq!(decoder.windows[0].pen_row, 3);
assert_eq!(decoder.windows[0].pen_column, 10);
Copy link
Member

Choose a reason for hiding this comment

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

Same in this file, some comments would help

@PunitLodha
Copy link
Member

If you find time you can write the extra comments, otherwise it's fine

@IshanGrover2004
Copy link
Contributor Author

@PunitLodha Added the CI & fixed the warnings as well

@PunitLodha PunitLodha merged commit f8001ae into CCExtractor:master Aug 11, 2024
15 of 16 checks passed
IshanGrover2004 added a commit to IshanGrover2004/ccextractor-fork that referenced this pull request Aug 23, 2024
* feat: Add new function to allocate any object to heap with zero allocated

* feat: Add unit tests for `decoder/commands.rs`

* docs: Mention about PR in changelogs

* feat: Add unit tests for `decoder/windows.rs`

Refactor the code and use Default where needed
Implement `PartialEq` also

* fix: Intialise tmp extern C values for easy mocking

* feat: Add unit tests for `decoder/timing.rs`

* feat: Add unit tests for `decoder/output.rs`

* feat: Add unit tests for `decoder/mod.rs`

* feat: Add unit tests for `decoder/tv_screen.rs`

* feat: Add unit tests for `lib.rs`

* fix: Failing test

* feat: [WIP] Add unit tests for `decoder/service_decoder.rs`

* feat: Add unit tests for `decoder/service_decoder.rs`

* feat: Add unit tests for `hardsubx/imgops.rs`

* feat: Add unit tests for `hardsubx/utility.rs`

* fix: cargo clippy

* fix: doctest for `lib_ccxr` module

* feat: Add test `lib_ccxr/util/mod.rs`

* feat: Add test `lib_ccxr/util/levenshtein.rs`

* feat: Add test `lib_ccxr/util/bits.rs`

* feat: Add test `lib_ccxr/time/units.rs`

* chore: Change function name

* fix: Failing of missing values `tlt_config`

* ci: Run unit test cases in `lib_ccxr` module also

* ci: Run clippy & fmt in `lib_ccxr` module also

* chore(clippy): Fix clippy warnings
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.

[PROPOSAL] Add some Unit Test for CCExtractor
3 participants