Amethyst: [MAINTENANCE] Exposing functionality for testing

[azriel91]

Description

While writing tests, I need some dummy Handle<_> values. However, since:

• Handle has no public constructor and is !Default.
• Handle#id is a private field.
• AssetStorage#allocate and allocate_new are not public.

There is no way to construct it without instantiating an amethyst application, which is rather heavy.

Having restricted visibility is good -- it prevents a footgun -- but I'm thinking it's okay to expose a public constructor to Handle, with accompanying documentation that explains that it should only be used in #[cfg(test)] circumstances.

Reason

Makes testing a lot more ergonomic, and also removes the need to initialize a whole amethyst application.

Impact

Do you think this change will have a negative impact on anything else?

no, shouldn't.

[randomPoison]

Might be good to take a look at some potential use cases for testing. i.e. in what sort of testing scenario would it be helpful to have a mock Handle? Concrete examples are often helpful for answering this sort of questions, in my experience.

[azriel91]

Oh true, should've included some examples.

• Example 1: umbrella type with multiple components, but only care about certain fields:

  struct Umbrella {
      something: Something,
      abc_handle: Handle<Abc>,
  }
  
  #[test]
  fn test_something() {
      let umbrella = Umbrella::new(something, /* .. */);
      assert!(use_something(umbrella).is_ok());
  }
  

• Example 2: I care that there is a Handle, but not its underlying value

  #[test]
  fn test_something() {
      let abcs = Vec::<Abc>::new();
      let handles = vec![Handle::new(0), Handle::new(1)];
      let my_type = MyType(abcs, handles);
  
      assert_eq!(2, my_type.max_component_len());
  }
  

[randomPoison]

It might make sense to have a way to create mock handles, rather than adding a constructor to Handle itself. Something like MockHandleFactory that could live separately in a test utilities module.

[stale[bot]]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed in 7 days if no further activity occurs. Feel free to comment if this ticket is still relevant.