add the notion of experimental dialects

[mtoy-googly-moogly]

1. A dialect can be marked "experimental"
2. Compiler will error if a connection with an experimental dialect is used without ##! experimental.dialect.DIALECT_NAME

This needs tests, which I will file as a next issue.

[christopherswenson]

I suspect a test for this will look like

1. Create a Runtime instance (with isTestRuntime = false)
2. Create a new dialect class with experimental = true
3. Call registerDialect to register the dialect with some unique name
4. Try to compile a model without the experiment enabled
5. Expect an error

Does it seem like that would work?

As a side note, I would love for options on the Runtime class to be specified when the runtime is constructed, rather than as a mutable property after the fact: new Runtime(urlReader, connections, { testEnvironment: true }). It also might be good if the flag here was more specific, since I don't know what else "testEnvironment" would control at the moment. I'm happy to make those changes later if you agree?

[mtoy-googly-moogly]

re: why property and not a constructor ... I had two minutes of confusion trying to figure out where runtimes come from, so it was safer for me to patch in the attribute when the runtime handed back to the test runner. I agree setting the flag at creation of the runtime makes more sense, I wasn't sure enough that I even had put the right pieces of data in the right objects to take the time to do it better, I have no objections to moving that piece of data to a constructor argument.

re: why a generic flag with side effects on the runtime, why not a specific flag I decided that a runtime needed to know if it was a test runtime, and the runtime could then decide, based on that flag, to instantiate a translator with any number of new behaviors. Even though that number is one right now, I felt like the Translator might have a suite of behaviors and thus was the proper place to hold the piece of state controlling that behavior, but a runtime only needed to know if it was a test runtime or not.

i like the plan for the test.

[mtoy-googly-moogly]

ok ... looking at the code and your suggestions ...

I 100% believe the flag on the Runtime should be "this is a test runtime".

it is important to know that not all runtimes are test runtimes, even in tests. The flag in the test runtimes is just another way (besides TestTranslator) of making sure that the instantiated Translator is aware that it is running tests.

The "testness" of a translator should probably be a constructor argument and it isn't one because of laziness.

[christopherswenson]

Started typing out some code but currently running into some build issues with duckdb....

WIP so far:

async function getError<T>(promise: Promise<T>): Promise<Error | undefined> {
  try {
    await promise;
  } catch (error) {
    return error;
  }
  return undefined;
}

describe('experimental dialects', () => {
  const connection = new DuckDBTestConnection(
    'duckdb',
    'test/data/duckdb/duckdb_test.db'
  );
  const runtime = testRuntimeFor(connection);
  runtime.isTestRuntime = false; // Enables checking experimental dialects

  class ExperimentalDuckdbDialect extends DuckDBDialect {
    experimental = true;
    name = 'duckdb_test_experimental';
  }

  registerDialect(new ExperimentalDuckdbDialect());

  test('generate an error when used without experiment enabled', async () => {
    const error = await getError(
      runtime.getModel(`
        source: s is duckdb_test_experimental.table('malloytest.aircraft')
      `)
    );
    expect(error).not.toBeUndefined();
    if (error !== undefined) {
      const problems = (error as MalloyError).problems;
      expect(problems.length).toBe(1);
      expect(problems[0].message).toContain('experimental');
    }
  });

  afterAll(async () => await runtime.connection.close());
});

[christopherswenson]

@mtoy-googly-moogly Here's a working(?) test, let me know what you think https://github.com/malloydata/malloy/pull/1745