Skip to content

refactor(test): refactor and delete redundant unit tests covered by main automation#100

Open
febyeji wants to merge 1 commit intomainfrom
feature/test-cleanup
Open

refactor(test): refactor and delete redundant unit tests covered by main automation#100
febyeji wants to merge 1 commit intomainfrom
feature/test-cleanup

Conversation

@febyeji
Copy link
Copy Markdown
Collaborator

@febyeji febyeji commented Apr 1, 2026

Summary

  • Most tests are subsumed by the previous merges.
  • Delete redundant unit tests covered by main automation

@febyeji febyeji force-pushed the feature/test-cleanup branch from 962b22b to ad247e6 Compare April 1, 2026 12:11
@tperami
Copy link
Copy Markdown
Collaborator

tperami commented Apr 3, 2026

I think some of those test are hard to maintain and subsumed by existing CLI-level test and #103

In general, I think unit tests need to

  • Only test small scale modules or functions that not reachable from the CLI
  • Only exists if we do not have to update them (non-automatically) all the time (which we can't do for ounit tests)

Any test at an entire test level should be done from the CLI as it is easier to maintain and forces use to keep in sync the whole stack.

That means concretely:

assert_parses_as "~(1:X0 = 1)"
           (Not (Atom (CmpCst (Reg (1, "X0"), Eq, Z.one))))
  • ir_test and testrepr_test are just testing whole file parsing, even if looking at individual fields with this PR.
    they seem somewhat maintainable so I guess we can leave them as they are unless/until they become annoying later

Pretty printing round-trip tests are good: They mean we can reparse what was printed, however they are not really a "unit" test so we might want to move them later

@febyeji febyeji changed the title refactor(test): replace inline TOML with existing test files refactor(test): delete redundant unit tests covered by main automation Apr 8, 2026
@febyeji febyeji changed the title refactor(test): delete redundant unit tests covered by main automation refactor(test): refactor and delete redundant unit tests covered by main automation Apr 8, 2026
@febyeji febyeji force-pushed the feature/test-cleanup branch from ad247e6 to 2ef6b90 Compare April 8, 2026 15:57
(******************************************************************************)

(** Unit tests for Isla.Assembler. *)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Those tests were good, why are you removing them?

(** Unit tests for Isla.Symbols. *)

open OUnit2

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Those were good

@febyeji febyeji force-pushed the feature/test-cleanup branch from 2ef6b90 to c07e534 Compare April 15, 2026 05:14
@febyeji febyeji force-pushed the feature/test-cleanup branch from c07e534 to 8a3cecd Compare April 15, 2026 08:36
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.

2 participants