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

Add tests #27

Merged
merged 28 commits into from
Jun 16, 2024
Merged

Add tests #27

merged 28 commits into from
Jun 16, 2024

Conversation

vixalien
Copy link
Collaborator

@vixalien vixalien commented Jan 17, 2024

Hello! This is a draft PR that adds comprehensive tests. These tests should be run on each PR to avoid regressions by adding a GitHub workflow. This can be done later. Here's the structure I'm suggesting for the tests/ folder:

  • unit/: This contains unit tests and should hopefully test each (exported) function in each file.
  • integration/: This should test that some processes work. Like for example, the require function or if the require functions caches namespaces. Or test unboxing and boxing of values.
  • regression/: This folder should be empty initially, but it should be used to add tests when there is a PR that fixes a specific issue so that the issue doesn't happen again. The files should be named according to the issue or PR number.
  • overrides/: This folder should be used to test the overrides in the related overrides folder (after Support overrides #10 is merged)
  • modules/: This should test the basic functionality of modules such as GLib.Bytes and Gtk.ListStore.
  • gi/: This should contain tests that will need additional .gir and library files to run (such as GIRegress and GIMarshalling)
  • etc...

However, I must admit that I'm not the best at doing test-related stuff, so this may not be the best approach.

Another option to consider is using a more flat directory layout and using periods (.) to structure tests, for example:

  • unit.base_utils.convert.ts
  • unit.base_utils.ffipp.ts
  • regression.2932.ts
  • overrides.GLib.ts
  • modules.Gtk.ts

As the tests need to be comprehensive, I will do my best to add various tests for each module, and then hopefully, it will get merged. Or we can add the initial tests boilerplate, and add tests as we go. In either case, I am longing for your feedback so I can know how to progress about this.

fixes #14

Unit tests:

  • src/base_utils/convert.ts
  • src/base_utils/ffipp.d.ts
  • src/base_utils/ffipp.js
  • src/base_utils/types.ts
  • src/bindings/enums.js
  • src/bindings/girepository.js
  • src/bindings/glib.js
  • src/bindings/mod.js
  • src/bindings/gobject.js
  • src/overrides/GObject.ts
  • src/overrides/mod.ts
  • src/types/argument/array.js
  • src/types/argument/interface.js
  • src/types/argument/list.js
  • src/types/callable/constructor.js
  • src/types/callable/vfunc.js
  • src/types/callable/function.js
  • src/types/callable/method.js
  • src/types/callback.js
  • src/types/constant.js
  • src/types/interface.js
  • src/types/prop.js
  • src/types/signal.js
  • src/types/struct.js
  • src/types/value.js
  • src/types/enum.js
  • src/types/object.js
  • src/types/callable.js
  • src/types/field.js
  • src/types/argument.js
  • src/utils/dataview.js
  • src/utils/error.ts
  • src/utils/gobject.js
  • src/utils/string.ts
  • src/gi.js
  • src/handleInfo.js

@ahgilak
Copy link
Owner

ahgilak commented May 25, 2024

Hello @vixalien
Are you still working on this?

@vixalien
Copy link
Collaborator Author

Are you still working on this?

Yes, very slowly 😭

@ahgilak
Copy link
Owner

ahgilak commented May 26, 2024

Yes, very slowly 😭

Great that it's still going. there's no rush :)
I suggest breaking this into multiple PRs so it would be easier to review and the tests which are ready get merged sooner.

@vixalien
Copy link
Collaborator Author

I suggest breaking this into multiple PRs so it would be easier to review and the tests which are ready get merged sooner.

Alright, I will make something mergeable soon for a few tests, then will add more tests as time passes. Finally we will need to add some integration tests.

One issue is how we will be able to run the "everything" tests. I'm not really that sure how we could build it. I will make a PR once it builds.

@vixalien
Copy link
Collaborator Author

Hi. I just add a simple meson configuration for the everything test, in short it does the following:

  1. downloads the gobject-introspection source
  2. builds libeverything
  3. builds the necessary GIR and typelib
  4. runs the test/everything/everything.ts file with the generated GIR and typelib
  5. we can now use gi://Everything

To run the tests, do the following:

meson setup build
cd build
ninja
# runs the `everything` tests
meson test everything

# you can also directly run the `everything.ts` file using the compiled `everything` library
ninja src/everything/everything.ts

In the future we can add a CI step, and add actual tests.

@vixalien
Copy link
Collaborator Author

We should also add the marshalling tests from https://github.com/GNOME/gobject-introspection/blob/main/tests/gimarshallingtests.c

we will get automatic regression tests for arrays/booleans/everything. With that set up, it will be more easy to experiment since we will be noticing when anything gets broken.

@vixalien
Copy link
Collaborator Author

vixalien commented Jun 9, 2024

Update: I've added a couple regress tests in tests/gi/regress adapted from https://gitlab.gnome.org/GNOME/gjs/-/blob/fe6ef0d9f6928521592e3d1558ef5caa5e23817b/installed-tests/js/testRegress.js

This will basically check against a library called Regress to ensure that we match the correct behavior. After this part is done, we will basically have "good" tests, even though I don't think all tests will pass initially.

To run them, just do:

meson setup build
cd build
ninja
# runs the `regress` tests
meson test regress

@vixalien vixalien force-pushed the tests branch 2 times, most recently from 47fadc6 to 86342f3 Compare June 10, 2024 02:02
@vixalien
Copy link
Collaborator Author

Hey, so I think this PR is partially ready. You can review it, and we will continue to implement the other parts from gitlab.gnome.org/GNOME/gjs/-/blob/fe6ef0d9f6928521592e3d1558ef5caa5e23817b/installed-tests/js/testRegress.js in other PRs so that we can iterate faster.

We will need to target the whole GIRegress and GIMarshalling APIs.

@vixalien vixalien marked this pull request as ready for review June 10, 2024 02:05
@vixalien vixalien changed the title Draft: Add tests Add tests Jun 10, 2024
@ahgilak
Copy link
Owner

ahgilak commented Jun 15, 2024

Are you able to run the examples now?

@vixalien
Copy link
Collaborator Author

Yes i'm able to run the examples. Aren't you able to?

@ahgilak
Copy link
Owner

ahgilak commented Jun 15, 2024

No I was getting errors.
I think it's fixed now

@vixalien
Copy link
Collaborator Author

Okay, now they are all working (except manual_loop.ts because of a typo)

@vixalien
Copy link
Collaborator Author

I fixed the various pre-existing regressions in #35 (which should be merged first)

@ahgilak ahgilak merged commit 9289337 into ahgilak:main Jun 16, 2024
@vixalien vixalien deleted the tests branch June 16, 2024 16:30
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.

Add tests
2 participants