Reviewdog #21
Open
Reviewdog #21
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Reestructuring the tests folder by moving unit tests to tests/unit will make KW more organized and prepared for the incoming integration tests. This commit also adds the flag `--unit` to `run_tests`, which limits the scope of the tests to unit test. This may not seem like a big deal yet becayse there are only unit tests, but it will be relevant once the integration tests are incorporated. Reviewed-by: Rodrigo Siqueira <[email protected]> Signed-off-by: uwla <[email protected]>
This commit only add {} around variables in a string concatenation.
Reviewed-by: David Tadokoro <[email protected]>
Signed-off-by: Rodrigo Siqueira <[email protected]>
Enable verbose for debug. Reviewed-by: David Tadokoro <[email protected]> Signed-off-by: Rodrigo Siqueira <[email protected]>
The `config --show` command now prints a header specifying the source of variables along with them. This requires the test to adapt to the new output: ignore the keywords GLOBAL/LOCAL in the output and expect the default scope to be empty. A note about the new default value for scope: If scope defaults to local, the configuration output would only show local variables. If defaults to global, it would only show global variables. But if defaults to empty, we can know the user has not specified an scope and therefore we should show both global and local configs. That's why its default value is now an empty string. Closes: kworkflow#894 Reviewed-by: David Tadokoro <[email protected]> Signed-off-by: uwla <[email protected]>
The name of checkpatch_wrapper.sh didn't fit with the project standard, so rename checkpatch_wrapper.sh to codestyle.sh, and rename checkpatch_wrapper_test.sh to codestyle_test.sh. Reviewed-by: Rodrigo Siqueira <[email protected]> Signed-off-by: André Gustavo Nakagomi Lopez <[email protected]> Co-authored-by: João Paulo Pereira <[email protected]>
The checkpatch_wrapper script was out of date in terms of code style, so modernize checkpatch_wrapper.sh to follow the current code standard. Changes include: - Use parse function to parse arguments, so now arguments work in any order - Update man page to include verbose option - Update autocomplete to include help and verbose options Issue kworkflow#935 Reviewed-by: Rodrigo Siqueira <[email protected]> Signed-off-by: André Gustavo Nakagomi Lopez <[email protected]> Co-authored-by: João Paulo Pereira da Silva <[email protected]>
Remove duplicated config entries in deploy config list. Reviewed-by: Rodrigo Siqueira <[email protected]> Signed-off-by: uwla <[email protected]>
While packaging kw for Debian, we got multiple warnings like this: kworkflow: bad-whatis-entry [usr/share/man/man1/kw-build.1.gz] This commit introduce the whatis for each kw feature. Reviewed-by: David Tadokoro <[email protected]> Signed-off-by: Rodrigo Siqueira <[email protected]>
With this new flag, the user can install KW without creating man pages, which takes time due to sphinx. This drastically reduce installation time and is specially useful in cases where the manual pages are not needed, such as local development and continuous integration. Reviewed-by: David Tadokoro <[email protected]> Signed-off-by: uwla <[email protected]>
The 'Registered/Unregistered Mailing Lists' screen has some missing mailing lists, due to the pagination of lore.kernel.org response. For example, the mailing list 'lvm-devel' doesn't appear in patch-hub because it is in the second page of lore.kernel.org. So, add pagination factor to display all the available mailing lists. Closes: kworkflow#901 Reviewed-by: David Tadokoro <[email protected]> Signed-off-by: André Gustavo Nakagomi Lopez <[email protected]> Co-authored-by: Joao Paulo Pereira da Silva <[email protected]>
The flag `--force` should never prompt during installation, but it does prompt in ArchLinux. Adding the flag `--noconfirm` to pacman's cmd fixes this issue. Closes kworkflow#983 Reviewed-by: David Tadokoro <[email protected]> Signed-off-by: uwla <[email protected]>
The `kw_config_loader` module would not read the option in the last line of a file if the line is missing the newline character '\n'. While it is a good practice to always add the newline character in text files, KW should still work in cases where it is missing, perhaps due to a mistake. Indeed, `kw init` creates config files some of which do not have the newline character in the last line (such as build.config) which is evidence that mistakes may happen, but even in such scenarios we want KW to function properly. Reviewed-by: David Tadokoro <[email protected]> Signed-off-by: uwla <[email protected]>
Add the following short flags: * -C for --skip-checks * -D for --skip-docs * -d for --docs * -f for --force * -r for --completely-remove * -t for --enable-tracing * -v for --verbose Usually short lowercase options enable some behavior. Thus, we use uppercase to sign we disable the behavior: the flag -D to disable creation of documentation as man pages, and the flag -C to disable dependencies checking. Closes kworkflow#993. Reviewed-by: Rodrigo Siqueira <[email protected]> Signed-off-by: uwla <[email protected]> Signed-off-by: Rodrigo Siqueira <[email protected]>
Test kw version on ArchLinux, Debian and Fedora using Podman. Also, change GitHub Unit Test Workflow to only run unit tests and ignore the integration tests. Reviewed-by: Rodrigo Siqueira <[email protected]> Co-authored-by: Fernando Henrique <[email protected]> Co-authored-by: Gustavo Ueti Fukunaga <[email protected]> Signed-off-by: uwla <[email protected]> Signed-off-by: Rodrigo Siqueira <[email protected]>
We mount a directory with sample config files in the container, then `run kw config --show <config>` and compare the result with the expected output. Reviewed-by: Rodrigo Siqueira <[email protected]> Co-authored-by: Fernando Henrique <[email protected]> Co-authored-by: Gustavo Ueti Fukunaga <[email protected]> Signed-off-by: uwla <[email protected]> Signed-off-by: Rodrigo Siqueira <[email protected]>
Test device info to check distribution on Archlinux, Debian and Fedora using podman. Reviewed-by: Rodrigo Siqueira <[email protected]> Co-authored-by: Fernando Henrique <[email protected]> Co-authored-by: Gustavo Ueti Fukunaga <[email protected]> Signed-off-by: uwla <[email protected]> Signed-off-by: Rodrigo Siqueira <[email protected]>
Provide documentation on integration tests, such as the dependencies required to run them. Signed-off-by: uwla <[email protected]> Reviewed-by: Rodrigo Siqueira <[email protected]> Signed-off-by: Rodrigo Siqueira <[email protected]>
The run_tests script used a `strip_path` function which would store the basename of all files in the global variable TESTS. When running the tests, we would use the find command to get the path of a file matching the basename of the current tests, then would check if that file exists and run the tests. The problem with this approach is that it assumes all tests have unique basename. This is no longer the case: we now have the `config_test.sh` integration test in addition to the `config_test.sh` unit test, and the same for device test. The old approach caused errors when attempting to run integration and unit tests with the same file Running `./run_tests.sh --integration test device` would work. Running `./run_tests.sh --unit test device` would work. Running `./run_tests.sh test device` would fail. To prevent this, this commit replaces the `strip_path` function with the `set_tests` function, which stores the path of the test instead of its basename. When running the tests, we use the `basename` command on the test path to print out the basename of the current test, rather than getting the path of the test from the basename of the test. Also, the new approach uses regex to match specified tests when running `./run_test.sh test <test1> <test2>`. That is because the find command cannot handle complex regex, only wildcards, so we use grep's to match multiple test files via the provided regex. Reviewed-by: Rodrigo Siqueira <[email protected]> Signed-off-by: uwla <[email protected]> Signed-off-by: Rodrigo Siqueira <[email protected]>
The device module depends on `lscpi` and `ps`, which are available via the packages `pciutils` and `procps`. Normally these dependencies will already be installed by default, but we cannot assume they are, so add them to the dependency lists. Closes kworkflow#1000 Reviewed-by: Rodrigo Siqueira <[email protected]> Signed-off-by: uwla <[email protected]> Signed-off-by: Rodrigo Siqueira <[email protected]>
To enable parallel processing of the patches kw used the slower option, /tmp/ directory, instead of /dev/shm/ directory. In this case, to speed up the processing, change the directory to /dev/shm. Also add unit test for the create_shared_memory_dir function. Reviewed-by: David Tadokoro <[email protected]> Signed-off-by: JGBSouza <[email protected]> Signed-off-by: David Tadokoro <[email protected]>
The unit tests `test_pre_process_xml_result` and `test_process_patchsets` had personal information embedded in sampled data. In this context, substitute the personal information in the samples by anonymous information based on famous musicians. Note: I took the opportunity to revise both unit tests, specially the `test_process_patchsets` one. Before, the `is_introduction_patch` and `extract_metadata_from_patch_title` functions were not mocked. Not mocking them made the unit test highly coupled with their implementation. Reviewed-by: David Tadokoro <[email protected]> Signed-off-by: JGBSouza <[email protected]> Signed-off-by: David Tadokoro <[email protected]>
Listing patches off any mailing list could result in duplicated patches being displayed unnecessarily. In this sense, solve the problem by checking if the patch already has been processed. Reviewed-by: David Tadokoro <[email protected]> Signed-off-by: JGBSouza <[email protected]> Signed-off-by: David Tadokoro <[email protected]>
Bookmarking patches didn't show any information about where the patchset was going to be downloaded. To solve this problem, add a checkbox message when the patchset is bookmarked or removed so the user can be sure where to find it or that it was removed. Reviewed-by: David Tadokoro <[email protected]> Signed-off-by: JGBSouza <[email protected]> Signed-off-by: David Tadokoro <[email protected]>
If the temporary directory already existed, it would not be cleaned up. This could cause issues if multiples deploys were made without rebooting the target. In this context, the code now removes the temporary directory and creates it again. Co-authored-by: uwla <[email protected]> Signed-off-by: João Gabriel Josephik <[email protected]> Reviewed-by: Rodrigo Siqueira <[email protected]> Signed-off-by: Rodrigo Siqueira <[email protected]>
When running on repo-mode, KW did not show up the correct version because it was simply running `cat` on a file. This works in normal installations because the `kw` installed version is copied to a static file during the installation. In repo-mode such information is dynamic, not static. In this case, it is necessary to run other commands. Also, call `kworkflow_function` in `setup.sh` to avoid code duplication. Before, `setup.sh` would run the same commands as in `src/help.sh`. Furthermore, add a unit test for kw version on repo mode. Closes kworkflow#986 Signed-off-by: uwla <[email protected]> Co-authored-by: João Gabriel Josephik <[email protected]> Reviewed-by: Rodrigo Siqueira <[email protected]> Signed-off-by: Rodrigo Siqueira <[email protected]>
Add unit tests for kw version under normal installation. Also, the repo-mode message is now sent to stderr. That way, it can be easily ignored with a simple redirection to /dev/null. This can be useful in future integration tests which may run kw in repo-mode. Otherwise, the output would always be messed up by the repo-mode message and have to be filtered out in every test. Signed-off-by: uwla <[email protected]> Signed-off-by: Rodrigo Siqueira <[email protected]> Reviewed-by: Rodrigo Siqueira <[email protected]>
Signed-off-by: Marcelo Mendes Spessoto Junior <[email protected]> Signed-off-by: Rodrigo Siqueira <[email protected]> Reviewed-by: Rodrigo Siqueira <[email protected]>
In fedora distro, when installing kw with setup.sh, it is always requested to install the package procps, even when it's already installed. The problem is that the command `rpm -q procps` seems to specifically not recognize that this package is already installed. However, with `rpm -q procps-ng` (which is the same exact package) it works fine. So, change the dependency to 'procps-ng' instead of 'procps' to fix this problem. Fixes: kworkflow#1010 Signed-off-by: João Paulo Pereira da Silva <[email protected]> Signed-off-by: Rodrigo Siqueira <[email protected]> Reviewed-by: Rodrigo Siqueira <[email protected]>
When manually configuring kw with `kw config`, it does not accept configurations with space separation, even with single ou double quotes put. For example, the command `kw config notification.sound_alert_command 'paplay berimbau.wav'` will result in the wrong configuration `notification.sound_alert_command=paplay` Because of that, change `kw config` to receive the value as all characters after the first space following configuration name, instead of just the first word. Fixes: kworkflow#1008 Signed-off-by: João Paulo Pereira da Silva <[email protected]> Reviewed-by: Rodrigo Siqueira <[email protected]> Signed-off-by: Rodrigo Siqueira <[email protected]>
Provide instructions for setting up podman to run in rootless mode. Also, fix a codeblock in `tests.rst` which was not correctly formatted and was displayed as inline instead of multiline. Signed-off-by: uwla <[email protected]> Signed-off-by: Rodrigo Siqueira <[email protected]> Reviewed-by: Rodrigo Siqueira <[email protected]>
The unit tests for from_sha feature compare output and result by using the compare_command_sequence function. Replace them with assertEquals, which is more appropriate for one-line output pairs. Reviewed-by: Rodrigo Siqueira <[email protected]> Signed-off-by: Marcelo Mendes Spessoto Junior <[email protected]> Signed-off-by: Rodrigo Siqueira <[email protected]>
Some users may like to follow the XDG Base Directory specification, and Zsh partially supports it as it allows the user to define environmental variables, such as ZDOTDIR, to override the default locations of dot files. With this commit, when there is no ~/.zshrc the setup script will now check if user has the ZDOTDIR directory defined and then if there is a .zshrc file inside. If it does, then the kw native Zsh completions necessary configs will be added to this file. Reviewed-by: David Tadokoro <[email protected]> Signed-off-by: Lincoln Yuji <[email protected]> Signed-off-by: David Tadokoro <[email protected]>
Correction to the SC2319 shellcheck warning about the possibility of overwriting the output value. To avoid this, the output value has been stored in a variable outside the if. Reviewed-by: David Tadokoro <[email protected]> Signed-off-by: Fernando Yang <[email protected]> Signed-off-by: David Tadokoro <[email protected]>
…options When running kw drm without options, it will show now the list of available options, providing more user-friendly experience. Reviewed-by: David Tadokoro <[email protected]> Signed-off-by: Fernando Yang <[email protected]> Signed-off-by: David Tadokoro <[email protected]>
…d rm Remove the short options -lm and -um, which don't work. Reviewed-by: David Tadokoro <[email protected]> Signed-off-by: Fernando Yang <[email protected]> Signed-off-by: David Tadokoro <[email protected]>
Add the return of success message, indicating the module was effectively loaded/unloaded. Add error return in load_module and unload_module if the module hasn`t been unloaded successfully. Closes: kworkflow#1002 Reviewed-by: David Tadokoro <[email protected]> Signed-off-by: Fernando Yang <[email protected]> Signed-off-by: David Tadokoro <[email protected]>
Add the `--repeat-previous` option for `kw pomodoro`, which allows the user to repeat the last Pomodoro session without typing it all over again. Closes: kworkflow#1009 Reviewed-by: David Tadokoro <[email protected]> Signed-off-by: Thiago Duvanel Ferreira <[email protected]> Signed-off-by: David Tadokoro <[email protected]>
Signed-off-by: Marcelo Mendes Spessoto Junior <[email protected]>
Signed-off-by: Marcelo Mendes Spessoto Junior <[email protected]>
Signed-off-by: Marcelo Mendes Spessoto Junior <[email protected]>
This commit refactors the integration test setup to dedicate a separate container for each test file. This change improves test environment management and reduces potential conflicts between tests. Reviewed-by: Rodrigo Siqueira <[email protected]> Signed-off-by: Aquila Macedo <[email protected]> Signed-off-by: Rodrigo Siqueira <[email protected]>
…oss podman versions Removed version-specific flags to ensure compatibility with different Podman versions. Updated teardown_single_container to use universally supported commands for stopping and removing containers. Tested on local machine with Podman version 4.3.1 and in GitHub Actions environment with Podman version 3.4.4. Reviewed-by: Rodrigo Siqueira <[email protected]> Signed-off-by: Aquila Macedo <[email protected]> Signed-off-by: Rodrigo Siqueira <[email protected]>
Signed-off-by: Marcelo Mendes Spessoto Junior <[email protected]>
Updated the run_tests script to default to unit tests if no specific type is provided. Improved variable naming for clarity (CLEAR_UNIT_CACHE, CLEAR_INTEGRATION_CACHE) and added --all flag to run all tests. Enhanced script logic for better maintainability and readability. Reviewed-by: Rodrigo Siqueira <[email protected]> Signed-off-by: Aquila Macedo <[email protected]>
Create build_and_deploy utility to centralize kw bd functionalities. Even though currently kw bd is only a "wrapper" for executing kw build and kw deploy, in the future it can have its own options. Therefore, this commit adds a file for kw bd along with a man page for it, making it easier for future enhancements to the command. Reviewed-by: Rodrigo Siqueira <[email protected]> Signed-off-by: Briza Mel Dias <[email protected]> Co-authored-by: Lorenzo Bertin Salvador <[email protected]> Signed-off-by: Rodrigo Siqueira <[email protected]>
This commit changes the name of the 'kw mail' feature to 'send-patch'. The motivation behind this change is that the name 'kw mail' was too generic and did not accurately represent the functionality of the feature. The 'kw mail' feature is designed to send patches via email, and its name should reflect this. The new name, 'send-patch', is more descriptive and immediately gives users an understanding of what the feature does. This change is important for improving the user experience and making the functionality of the features clear at first glance. Reviewed-by: Rodrigo Siqueira <[email protected]> Signed-off-by: Lucas Quaresma <[email protected]> Co-authored-by: Roberto Bolgheroni <[email protected]> Signed-off-by: Rodrigo Siqueira <[email protected]>
Provide information when using 'kw device' command to help developers. In this sense, adding kernel info seems appropriate. Added kernel name, release, version and machine hardware name. Reviewed-by: Rodrigo Siqueira <[email protected]> Signed-off-by: Gustavo Rodrigues <[email protected]> Signed-off-by: Rodrigo Siqueira <[email protected]>
Provide more information when using 'kw device' command to help developers. In this sense, adding cpu architecure info seems appropriate. Reviewed-by: Rodrigo Siqueira <[email protected]> Signed-off-by: Gustavo Rodrigues <[email protected]> Signed-off-by: Rodrigo Siqueira <[email protected]>
Signed-off-by: Marcelo Mendes Spessoto Junior <[email protected]>
…unction Ensures better debugging and error reporting by capturing the command that caused the failure. Reviewed-by: Rodrigo Siqueira <[email protected]> Signed-off-by: Aquila Macedo <[email protected]> Signed-off-by: Rodrigo Siqueira <[email protected]>
… run entirely in container This commit modifies the device_test integration test to run entirely within a container. The changes include restructuring the test to use a results array to store the output of the `kw device --local` command from Debian, Fedora, and Arch distributions. New helper functions were added to handle the setup and execution of tests for RAM, storage, OS, motherboard, chassis, CPU, and GPU information. Minor adjustments were made to escape single quotes in the `container_exec` function to ensure proper command execution. Reviewed-by: Rodrigo Siqueira <[email protected]> Signed-off-by: Aquila Macedo <[email protected]> Signed-off-by: Rodrigo Siqueira <[email protected]>
Signed-off-by: Marcelo Mendes Spessoto Junior <[email protected]>
Signed-off-by: Marcelo Mendes Spessoto Junior <[email protected]>
Signed-off-by: Marcelo Mendes Spessoto Junior <[email protected]>
Signed-off-by: Marcelo Mendes Spessoto Junior <[email protected]>
Signed-off-by: Marcelo Mendes Spessoto Junior <[email protected]>
Signed-off-by: Marcelo Mendes Spessoto Junior <[email protected]>
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| setup_for_symbolic_link_test | ||
|
|
||
| [[ -L "$symlink" ]] | ||
| assert_equals_helper 'Symbolic link was not created' "$LINENO" 0 "$?" |
| setup_for_symbolic_link_test | ||
|
|
||
| [[ -L "$symlink" ]] | ||
| assert_equals_helper 'Symbolic link was not created' "$LINENO" 0 "$?" |
| setup_for_symbolic_link_test | ||
|
|
||
| [[ -L "$symlink" ]] | ||
| assert_equals_helper 'Symbolic link was not created' "$LINENO" 0 "$?" |
| setup_for_symbolic_link_test | ||
|
|
||
| [[ -L "$symlink" ]] | ||
| assert_equals_helper 'Symbolic link was not created' "$LINENO" 0 "$?" |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.