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

Test framework to support running single tests #766

Open
nocodehummel opened this issue Apr 16, 2024 · 7 comments
Open

Test framework to support running single tests #766

nocodehummel opened this issue Apr 16, 2024 · 7 comments
Labels
enhancement New feature or request

Comments

@nocodehummel
Copy link
Contributor

Feature Request

The test framework should be more flexible and be able to run specific tests independently. Currently it is not possible to run a specific test with it.only or using a tool like Jest Runner in VSC.

Use Cases

While investigating an issue it is very difficult to make incremental progress and verify the outcome with a test case. This was especially complex related to #764.

The goal would be for developers to be able to contribute to the library with test driven development. Without the support of test cases it is hard to make changes in a complex library. The test cases are the ideal way to trigger specific functions and verify results.

Other

The current approach with fetchDevel causes inter-dependencies between actual application modules and test functions. These dependencies cause issues in specific environments (#645) and with loading modules when running a single test.

SyntaxError: Cannot use 'import.meta' outside a module

       8 |   if (fetchDevelFunc) return fetchDevelFunc;
       9 |
    > 10 |   const module = await import("./lib/fetchDevel.js");
         |                  ^
      11 |   fetchDevelFunc = module.default;
      12 |   return fetchDevelFunc;
      13 | }

A redesign to use JEST Class Mocks could be a possible solution.

Related topic to this is the use of test data. In principle it should be possible to delete the cached data and than successfully run the tests. This is currently difficult as the cached data is mixed with other (manipulated) test data for specific cases.

I wanted to share my thoughts after spending some time investigating an issue.

@nocodehummel nocodehummel added the enhancement New feature or request label Apr 16, 2024
@gadicc
Copy link
Owner

gadicc commented Apr 17, 2024

Hey @nocodehummel, thanks for the above feedback and in general for your contributions to the project.

I'll note that in the console at least, you can target specific files quite easy, i.e.

$ yarn test getCrumb

which I think is just a filename glob but could possibly look through test names too, I never actually looked into it further.

I'm surprised things like it.only doesn't work and we definitely need to look into that. I've never looked at VSC's Jest Runner but I would definitely love this to work too, especially we are using jest under the hood. I don't know if maybe it has to do with all the special options we need for jest, as listed in the package.json scripts section.

As for your point on fetchDevel convoluting things, and jest mocks being a better approach, I agree 💯.

In general, the caching approach taken by fetchDevel works so well that I did eventually turn it into it's own package, using a jest mocking approach, that has been a dream to use in other packages: https://www.npmjs.com/package/jest-fetch-mock-cache

I would love to eventually rewrite the test code in yahoo-finance2 to instead use the above package, I just don't know when I'll have the time for it :/ I think it will be quite a lot of work to change the flow and deal with various edge-cases we currently handle with fetchDevel.

@nocodehummel
Copy link
Contributor Author

@gadicc thanks for the quick response.
Given your positive feedback to improve the framework I can start with a PoC on some part of the code.
And get your feedback on that. I will have a look at the caching package for this.
I do agree that it probably is a lot of work, but should make life easier in the long run.

@gadicc
Copy link
Owner

gadicc commented Apr 17, 2024

@nocodehummel, my pleasure, and thanks for all your help with the project :)

Sure, that would be amazing! I really do struggle with time for the project, so this kind of help is super appreciated. Having said that, if this particular issue ends up being more work than you expected and you change your mind, please don't worry about it. But otherwise yes, I agree completely; long term maintainability and contributability for the win! 🙏

@nocodehummel
Copy link
Contributor Author

@gadicc, a first test case with the jest-fetch-mock-cache package created an empty cached file upon response with status 307. My assumption was that the file would contain a cached response object. Is this the expected behavior?

image

@nocodehummel
Copy link
Contributor Author

nocodehummel commented Apr 18, 2024

@gadicc I made a minor change to the fetch caching package by saving without header in the file name.
Just saving by URL name work fine. What is the use-case for the header in the file name?

@gadicc
Copy link
Owner

gadicc commented Apr 18, 2024

Hey! Oh wow, you don't waste any time... thanks! 😁

I won't be at my PC most of today but let me try answer briefly before I head out.

jest-fetch-mock-cache package created an empty cached file upon response with status 307. My assumption was that the file would contain a cached response object. Is this the expected behavior?

Sorry, I think all the dev work I did on the package until now assumed a 200 status, so yes, this looks like an unfortunate bug to discover so early. Your assumption is correct on the expected behaviour, I should have a chance to look at this tomorrow if you don't beat me to it :) We'll add a test case for redirects.

I made a minor change to the fetch caching package by saving without header in the file name.
Just saving by URL name work fine. What is the use-case for the header in the file name?

The issue here is that the request headers also affect the response. Case in point, the user-agent for /v1/test/getcrumb, or cookies. So, without the header hash to differentiate, in the getcrumb example, both the working and unworking case would have had the same filename and would have incorrectly returned the same result.

I may not be around again until tomorrow but will check my phone when I can. Thanks again for your efforts here 🙏

@nocodehummel
Copy link
Contributor Author

See gadicc/jest-fetch-mock-cache#3.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants