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

eunit: exact execution option #6157

Merged
merged 1 commit into from
Sep 5, 2022
Merged

eunit: exact execution option #6157

merged 1 commit into from
Sep 5, 2022

Conversation

u3s
Copy link
Contributor

@u3s u3s commented Jul 15, 2022

  • option for avoiding automatic execution of a module with "_tests" suffix

@u3s u3s self-assigned this Jul 15, 2022
@u3s u3s added the team:PS Assigned to OTP team PS label Jul 15, 2022
@github-actions
Copy link
Contributor

github-actions bot commented Jul 15, 2022 •
edited
Loading

CT Test Results

    2 files    12 suites   3m 10s ⏱️
100 tests   98 ✔️ 2 💤 0 ❌
115 runs  113 ✔️ 2 💤 0 ❌

Results for commit dd366cb.

♻️ This comment has been updated with latest results.

To speed up review, make sure that you have read Contributing to Erlang/OTP and that all checks pass.

See the TESTING and DEVELOPMENT HowTo guides for details about how to run test locally.

Artifacts

// Erlang/OTP Github Action Bot

@u3s
Copy link
Contributor Author

u3s commented Jul 15, 2022

  • @vkatsuba alternative approach compared to Delete duplicates of unit tests #5461
  • intention is to leave legacy execution sequence unchanged
  • but automated execution of "_tests" module can be disabled if not expected (such as when m and m_tests modules are placed in src folder and execution of all modules from folder is started, I observed this with rebar3 today :-) so I decided it is high time to come back to this one :-)) )

@u3s u3s force-pushed the kuba/eunit/add_exact_execution_option/OTP-18181 branch 3 times, most recently from ebb1cda to a272af7 Compare July 19, 2022 09:44
KennethL
KennethL reviewed Jul 27, 2022
View reviewed changes
lib/eunit/src/eunit.erl Outdated Show resolved Hide resolved
lib/eunit/src/eunit_data.erl Show resolved Hide resolved
IngelaAndin
IngelaAndin reviewed Jul 28, 2022
View reviewed changes
lib/eunit/src/eunit_data.erl Outdated
@@ -573,22 +574,24 @@ get_module_tests(M) ->
throw({module_not_found, M})
end.

get_module_tests_1(M, Es) ->
get_module_tests_1(M, Es, Options) ->
Copy link
Contributor

Choose a reason for hiding this comment

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

So I know this is not part of your change, just an observation that the very short variable names make the code hard to read I think. A few more characters that would make it easier to get the correct association without analyzing the context could be beneficial. It could perhaps be an enhancement if you think it is worth the time.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I agree with observation. Unfortunately this is a style eunit code is written :-/ There is limited number of tests of application as well.

I will see what can be improved for eunit_data module - assuming some time box for activity.

Copy link
Contributor Author

@u3s u3s Jul 28, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

I've pushed a commit please have a look and let me know what you think
IMHO, result is a better clarity of the code (at least for non eunit expert) but it is costly to implement and then review.
Not to mention the risk of messing up with logic accidentally and releasing it due to limited test coverage ...

@u3s u3s mentioned this pull request Sep 2, 2022
Closed
@u3s
eunit: exact_execution option
c194380 
- option for avoiding automatic execution of a module with "_tests" suffix
@u3s u3s force-pushed the kuba/eunit/add_exact_execution_option/OTP-18181 branch from dd366cb to c194380 Compare September 2, 2022 07:38
@u3s u3s linked an issue Sep 2, 2022 that may be closed by this pull request
ERL-97: eunit run twice tests when both module and module_tests given #3064
Closed
@u3s u3s merged commit a4c4e23 into erlang:maint Sep 5, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@KennethL KennethL KennethL left review comments

@IngelaAndin IngelaAndin IngelaAndin approved these changes

Assignees

@u3s u3s

Labels
in progress team:PS Assigned to OTP team PS
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

ERL-97: eunit run twice tests when both module and module_tests given
3 participants
@u3s @KennethL @IngelaAndin