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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

feature: Disable eager service loading for tests #380

Open
South-Paw opened this issue Jun 28, 2021 · 3 comments
Open

feature: Disable eager service loading for tests #380

South-Paw opened this issue Jun 28, 2021 · 3 comments
Labels
flag: needs discussion Issues which needs discussion before implementation. type: feature Issues related to new features.

Comments

@South-Paw
Copy link

South-Paw commented Jun 28, 2021

Description

I started using the eager flag on some of my @Services that I would expect to be started as soon as they are registered (Database, Logger, Mailer, etc) which works great 馃憤

However when unit testing, these services are registering themselves during tests that don't require them and then beginning their start up routines (initializing connections etc) which causes tests to fail or have other issues

Proposed solution

Would it be possible to set some configuration on the Container to instruct it to ignore eager services when testing?

Other alternatives I've tried/thought about

  • Mocking the eager services in a setupTests file (however this didn't seem to work, they are still registered then starting up)
  • I suppose I could remove the this.start() calls from the service's constructors, but then I'll have to get them from the container to manually call start() on them (which sort of defeats the purpose of the eager 馃槃)

Thanks

@South-Paw South-Paw added flag: needs discussion Issues which needs discussion before implementation. type: feature Issues related to new features. labels Jun 28, 2021
@South-Paw South-Paw changed the title feature: Disable eager service loading for tests feature: Disable eager service loading for tests Jun 28, 2021
@South-Paw
Copy link
Author

South-Paw commented Jun 28, 2021

Sort of related to the conversation in #126

If its relevant, the services I'm starting using eager look very similar to the following...

@Service({ eager: true })
export class EngineImpl {
  private readonly engine;

  constructor() {
    // this may be a database connection or a mailer transport in the real-world
    this.engine = createEngine();

    this.start();
  }

  private async start() {
    console.log("start your engines...");

    try {
      if (await this.engine.start()) {
        console.log("engine goes brrrrrr");
      }
    } catch (err) {
      console.log("oops the engine stalled, retrying in 3 seconds...");

      setTimeout(() => this.start(), 3000);
    }
  }
}

@NoNameProvided
Copy link
Member

However when unit testing, these services are registering themselves during tests that don't require them and then beginning their start up routines (initializing connections etc) which causes tests to fail or have other issues

How do you export these services? Unit tests should import it tests only. So your eager services should not be imported at all if you are not specifically testing them.

freshgum-bubbles added a commit to freshgum-bubbles/typedi that referenced this issue Jun 24, 2023
Eager services are harder to test, and
make application behaviour more brittle.
As a result, I do personally discourage
using them.
Therefore, by default they are now disabled.

This will need more testing.

Some further remarks below:
Eager loading is generally discouraged, as it makes testing harder
and makes the application more confusing to work with.
For instance, see:
  [typestack/typedi#380](typestack/typedi#380).

Consider an example of a DatabaseService with eager loading enabled.
Once imported, database connections and more will
immediately start taking place.

In many cases, this will be unexpected and will
ultimately be an unwanted side effect.

Therefore, we place the eager loading functionality
behind a toggleable option,
which must be enabled prior to any eager loading
strategies taking place.
freshgum-bubbles added a commit to freshgum-bubbles/typedi that referenced this issue Jun 24, 2023
Eager services are harder to test, and
make application behaviour more brittle.
As a result, I do personally discourage
using them.
Therefore, by default they are now disabled.

This will need more testing.

Some further remarks below:
Eager loading is generally discouraged, as it makes testing harder
and makes the application more confusing to work with.
For instance, see:
  [typestack/typedi#380](typestack/typedi#380).

Consider an example of a DatabaseService with eager loading enabled.
Once imported, database connections and more will
immediately start taking place.

In many cases, this will be unexpected and will
ultimately be an unwanted side effect.

Therefore, we place the eager loading functionality
behind a toggleable option,
which must be enabled prior to any eager loading
strategies taking place.
freshgum-bubbles added a commit to freshgum-bubbles/typedi that referenced this issue Jun 24, 2023
Eager services are harder to test, and
make application behaviour more brittle.
As a result, I do personally discourage
using them.
Therefore, by default they are now disabled.

This will need more testing.

Some further remarks below:
Eager loading is generally discouraged, as it makes testing harder
and makes the application more confusing to work with.
For instance, see:
  [typestack/typedi#380](typestack/typedi#380).

Consider an example of a DatabaseService with eager loading enabled.
Once imported, database connections and more will
immediately start taking place.

In many cases, this will be unexpected and will
ultimately be an unwanted side effect.

Therefore, we place the eager loading functionality
behind a toggleable option,
which must be enabled prior to any eager loading
strategies taking place.

feat(ContainerInstance): disable eager services by default

Eager services are harder to test, and
make application behaviour more brittle.
As a result, I do personally discourage
using them.
Therefore, by default they are now disabled.

This will need more testing.

Some further remarks below:
Eager loading is generally discouraged, as it makes testing harder
and makes the application more confusing to work with.
For instance, see:
  [typestack/typedi#380](typestack/typedi#380).

Consider an example of a DatabaseService with eager loading enabled.
Once imported, database connections and more will
immediately start taking place.

In many cases, this will be unexpected and will
ultimately be an unwanted side effect.

Therefore, we place the eager loading functionality
behind a toggleable option,
which must be enabled prior to any eager loading
strategies taking place.

feat(ContainerInstance): disable eager services by default

Eager services are harder to test, and
make application behaviour more brittle.
As a result, I do personally discourage
using them.
Therefore, by default they are now disabled.

This will need more testing.

Some further remarks below:
Eager loading is generally discouraged, as it makes testing harder
and makes the application more confusing to work with.
For instance, see:
  [typestack/typedi#380](typestack/typedi#380).

Consider an example of a DatabaseService with eager loading enabled.
Once imported, database connections and more will
immediately start taking place.

In many cases, this will be unexpected and will
ultimately be an unwanted side effect.

Therefore, we place the eager loading functionality
behind a toggleable option,
which must be enabled prior to any eager loading
strategies taking place.
freshgum-bubbles added a commit to freshgum-bubbles/typedi that referenced this issue Jun 25, 2023
Eager services are harder to test, and
make application behaviour more brittle.
As a result, I do personally discourage
using them.
Therefore, by default they are now disabled.

This will need more testing.

Some further remarks below:
Eager loading is generally discouraged, as it makes testing harder
and makes the application more confusing to work with.
For instance, see:
  [typestack/typedi#380](typestack/typedi#380).

Consider an example of a DatabaseService with eager loading enabled.
Once imported, database connections and more will
immediately start taking place.

In many cases, this will be unexpected and will
ultimately be an unwanted side effect.

Therefore, we place the eager loading functionality
behind a toggleable option,
which must be enabled prior to any eager loading
strategies taking place.
freshgum-bubbles added a commit to freshgum-bubbles/typedi that referenced this issue Jun 25, 2023
Eager services are harder to test, and
make application behaviour more brittle.
As a result, I do personally discourage
using them.
Therefore, by default they are now disabled.

This will need more testing.

Some further remarks below:
Eager loading is generally discouraged, as it makes testing harder
and makes the application more confusing to work with.
For instance, see:
  [typestack/typedi#380](typestack/typedi#380).

Consider an example of a DatabaseService with eager loading enabled.
Once imported, database connections and more will
immediately start taking place.

In many cases, this will be unexpected and will
ultimately be an unwanted side effect.

Therefore, we place the eager loading functionality
behind a toggleable option,
which must be enabled prior to any eager loading
strategies taking place.
@freshgum-bubbles
Copy link

(yikes, sorry about the spam: I had an issue with Git and had to force push it a few times, didn't realise linking the issue would notify on every commit...)

Mocking the eager services in a setupTests file (however this didn't seem to work, they are still registered then starting up)

The way I did this in my TypeDI fork was turning off eager initialisation by default, and having it enable-able by a flag. That way, you can easily enable it in your application code prior to importing any eager services, but by default it's disabled -- this makes it much easier to test.

It's enabled by a enableEagerLoading flag which tells the container to eagerly run any eager services it encounters. However, you may want to implement a buffer of eager services: if the flag wasn't enabled when the eager service was registered, it'll be added to a buffer and, if enableEagerLoading is then called, that buffer will be flushed and each service within would be passed to get.

https://github.com/freshgum-bubbles/typedi/blob/c3d4f88328ac1b18962515c6ce90db300a3f273b/src/container-instance.class.ts#L957-L959

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
flag: needs discussion Issues which needs discussion before implementation. type: feature Issues related to new features.
Development

No branches or pull requests

3 participants